Skip to content
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

Merged
merged 3 commits into from
Nov 6, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,18 @@ module.exports = {
// This should create two streams of documentLoad sequences, all with the same sessionId but having
// two scriptInstances (one from parent, one from iframe)
const parent = await browser.globals.findSpan(span => span.name === 'documentFetch' && span.tags['location.href'].includes('cookies.ejs'));
await browser.assert.ok(parent.tags['splunk.rumSessionId']);
await browser.assert.ok(parent.tags['browser.instance.id']);

await browser.assert.notEqual(parent.tags['splunk.scriptInstance'], parent.tags['splunk.rumSessionId']);

const iframe = await browser.globals.findSpan(span => span.name === 'documentFetch' && span.tags['location.href'].includes('iframe.ejs'));
await browser.assert.ok(iframe.tags['splunk.rumSessionId']);
await browser.assert.notEqual(iframe.tags['splunk.scriptInstance'], iframe.tags['splunk.rumSessionId']);

// same session id
// same session id & instanceId
await browser.assert.equal(parent.tags['splunk.rumSessionId'], iframe.tags['splunk.rumSessionId']);
await browser.assert.equal(parent.tags['browser.instance.id'], iframe.tags['browser.instance.id']);

// but different scriptInstance
await browser.assert.notEqual(parent.tags['splunk.scriptInstance'], iframe.tags['splunk.scriptInstance']);

Expand Down
1 change: 1 addition & 0 deletions packages/web/src/SplunkSpanAttributesProcessor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ export class SplunkSpanAttributesProcessor implements SpanProcessor {
span.setAttribute('location.href', location.href);
span.setAttributes(this._globalAttributes);
span.setAttribute('splunk.rumSessionId', getRumSessionId());
span.setAttribute('browser.instance.visibility_state', document.visibilityState);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice

Copy link
Member Author

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.

}

onEnd(): void {
Expand Down
2 changes: 2 additions & 0 deletions packages/web/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ import { SessionBasedSampler } from './SessionBasedSampler';
import { SocketIoClientInstrumentationConfig, SplunkSocketIoClientInstrumentation } from './SplunkSocketIoClientInstrumentation';
import { SplunkOTLPTraceExporter } from './exporters/otlp';
import { registerGlobal, unregisterGlobal } from './global-utils';
import { BrowserInstanceService } from './services/BrowserInstanceService';

export { SplunkExporterConfig } from './exporters/common';
export { SplunkZipkinExporter } from './exporters/zipkin';
Expand Down Expand Up @@ -433,6 +434,7 @@ export const SplunkRum: SplunkOtelWebType = {
// Splunk specific attributes
'splunk.rumVersion': VERSION,
'splunk.scriptInstance': instanceId,
'browser.instance.id': BrowserInstanceService.id,
'app': applicationName,
};

Expand Down
53 changes: 53 additions & 0 deletions packages/web/src/services/BrowserInstanceService.ts
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;
Copy link
Contributor

Choose a reason for hiding this comment

The 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 scriptInstance, tricking you into believing that these are separate windows.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Please check 861bed8


return this._id;
}
}
37 changes: 37 additions & 0 deletions packages/web/src/utils/storage.ts
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);
Copy link
Contributor

@jtmalinowski jtmalinowski Nov 4, 2024

Choose a reason for hiding this comment

The 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:

survives over page reloads and restores.

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?

Duplicating a tab copies the tab's sessionStorage into the new tab.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

survives over page reloads and restores

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.

Duplicating a tab copies the tab's sessionStorage into the new tab.

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;
}
};
Loading