-
Notifications
You must be signed in to change notification settings - Fork 31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: attach browser.instance.id
and browser.instance.visibility_state
to spans
#878
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
/* | ||
Copyright 2024 Splunk Inc. | ||
|
||
Licensed under the Apache License, Version 2.0 (the "License"); | ||
you may not use this file except in compliance with the License. | ||
You may obtain a copy of the License at | ||
|
||
http://www.apache.org/licenses/LICENSE-2.0 | ||
|
||
Unless required by applicable law or agreed to in writing, software | ||
distributed under the License is distributed on an "AS IS" BASIS, | ||
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
See the License for the specific language governing permissions and | ||
limitations under the License. | ||
*/ | ||
|
||
import { safelyGetSessionStorage, safelySetSessionStorage } from '../utils/storage'; | ||
import { generateId } from '../utils'; | ||
|
||
const BROWSER_INSTANCE_ID_KEY = 'browser_instance_id'; | ||
|
||
/** | ||
* BrowserInstanceService is responsible for generating and storing a unique ID for the tab. | ||
* THe ID is stored in the session storage, so stays the same even after page reload | ||
* as long as the tab stays on the same domain. | ||
* This ID will be the same for all frames/context in the tab as long as they are on the same domain. | ||
* Currently, this is simplified version which has this limitation: | ||
* - It does not cover the case when tab is duplicated - | ||
* browsers copy storage when duplicating tabs so the ID will be the same | ||
* To cover this case we need to implement a communication between tabs which requires asynchronous initialization. | ||
* This is not implemented yet as requires bigger refactoring. | ||
*/ | ||
export class BrowserInstanceService { | ||
static _id: string | undefined = undefined; | ||
|
||
static get id(): string { | ||
if(this._id) { | ||
return this._id; | ||
} | ||
|
||
|
||
// Check if the ID is already stored in the session storage. It might be generated by another frame/context. | ||
let browserInstanceId = safelyGetSessionStorage(BROWSER_INSTANCE_ID_KEY); | ||
if(!browserInstanceId) { | ||
browserInstanceId = generateId(64); | ||
safelySetSessionStorage(BROWSER_INSTANCE_ID_KEY, browserInstanceId); | ||
} | ||
|
||
this._id = browserInstanceId; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if we didn't manage to persist it then wouldn't it be better to not attach it to spans? Because now let's say someone blocks session storage, and we see their session, it'll have a different browser.instance.id for each There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point. Please check 861bed8 |
||
|
||
return this._id; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
/* | ||
Copyright 2024 Splunk Inc. | ||
|
||
Licensed under the Apache License, Version 2.0 (the "License"); | ||
you may not use this file except in compliance with the License. | ||
You may obtain a copy of the License at | ||
|
||
http://www.apache.org/licenses/LICENSE-2.0 | ||
|
||
Unless required by applicable law or agreed to in writing, software | ||
distributed under the License is distributed on an "AS IS" BASIS, | ||
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
See the License for the specific language governing permissions and | ||
limitations under the License. | ||
*/ | ||
|
||
export const safelyGetSessionStorage = (key: string): string | null => { | ||
let value = null; | ||
try { | ||
value = window.sessionStorage.getItem(key); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. okay on the topic of session storage I don't have any strong feelings either way and it seems alright but following MDN's docs:
Page reloads I think we can accept because they'll come up in spans. Restores seem like a rare case. So I think this is okayish?
Not sure about this one but it also seems rare enough? I think we can just deploy this and check if it's good enough in practice. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I did it intentionally that way. It is representing the browser tab rather than just the current window. I mean it can be adjusted if needed.
I guess it is the same as the "restore" case. In the Cisco agent, we cover these use cases, but it requires a bigger refactor as the initialization would need to be asynchronous, and we would need to postpone span sending until the ID is assigned. It can be built on top of this as it is just an additional improvement. |
||
} catch { | ||
// sessionStorage not accessible probably user is in incognito-mode | ||
// or set "Block third-party cookies" option in browser settings | ||
} | ||
return value; | ||
}; | ||
|
||
export const safelySetSessionStorage = (key: string, value: string): boolean => { | ||
try { | ||
window.sessionStorage.setItem(key, value); | ||
return true; | ||
} catch { | ||
// sessionStorage not accessible probably user is in incognito-mode | ||
// or set "Block third-party cookies" option in browser settings | ||
return false; | ||
} | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can now filter how many sessions consist of only long tasks in a hidden state.