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.

Sign up for GitHub

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

Jump to bottom

feat(instrumentation-document-load): Add ability to disable span events #2188

Merged
merged 13 commits into from
May 30, 2024
Merged

feat(instrumentation-document-load): Add ability to disable span events #2188

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
c5a95ee
inital try at disabling span events option
pkanal May 8, 2024
bc06210
fix bugs with tdd
pkanal May 9, 2024
e3e5f3c
remove .only from tests
pkanal May 9, 2024
a51b8a7
fix lint
pkanal May 15, 2024
33ee0ad
Merge branch 'main' into feat/user-interaction-instrumentation-span-e…
pkanal May 15, 2024
5231920
Merge branch 'main' into feat/user-interaction-instrumentation-span-e…
JamieDanielson May 16, 2024
f6a3257
Merge branch 'main' into feat/user-interaction-instrumentation-span-e…
pkanal May 17, 2024
99b090c
rename to `ignorePerformancePaintEvents`
pkanal May 21, 2024
860c584
add documentation
pkanal May 21, 2024
c61132c
Merge branch 'main' into feat/user-interaction-instrumentation-span-e…
pkanal May 21, 2024
564a7d4
Merge branch 'main' into feat/user-interaction-instrumentation-span-e…
pkanal May 23, 2024
a4c1923
Merge branch 'main' into feat/user-interaction-instrumentation-span-e…
pkanal May 30, 2024
8b6d9db
Merge branch 'main' into feat/user-interaction-instrumentation-span-e…
pkanal May 30, 2024
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
12 changes: 12 additions & 0 deletions plugins/web/opentelemetry-instrumentation-document-load/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,18 @@ registerInstrumentations({

See [examples/tracer-web](https://github.com/open-telemetry/opentelemetry-js/tree/main/examples/tracer-web) for a short example.

## Document Load Instrumentation Options

The document load instrumentation plugin has few options available to choose from. You can set the following:

| Options | Type | Description |
|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------|-------------------------------|-----------------------------------------------------------------------------------------|
| `applyCustomAttributesOnSpan.documentLoad`| `DocumentLoadCustomAttributeFunction` | Function for adding custom attributes to `documentLoad` spans. |
| `applyCustomAttributesOnSpan.documentFetch` | `DocumentLoadCustomAttributeFunction` | Function for adding custom attributes to `documentFetch` spans. |
| `applyCustomAttributesOnSpan.resourceFetch` | `ResourceFetchCustomAttributeFunction` | Function for adding custom attributes to `resourceFetch` spans |
| `ignoreNetworkEvents` | `boolean` | Ignore adding [network events as span events](https://github.com/open-telemetry/opentelemetry-js/blob/e49c4c7f42c6c444da3f802687cfa4f2d6983f46/packages/opentelemetry-sdk-trace-web/src/enums/PerformanceTimingNames.ts#L17) for document fetch and resource fetch spans. |
| `ignorePerformancePaintEvents` | `boolean` | Ignore adding performance resource paint span events to document load spans. |

## Semantic Conventions

This package uses `@opentelemetry/semantic-conventions` version `1.22+`, which implements Semantic Convention [Version 1.7.0](https://github.com/open-telemetry/opentelemetry-specification/blob/v1.7.0/semantic_conventions/README.md)
Expand Down
45 changes: 29 additions & 16 deletions plugins/web/opentelemetry-instrumentation-document-load/src/instrumentation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,9 @@ export class DocumentLoadInstrumentation extends InstrumentationBase {
if (fetchSpan) {
fetchSpan.setAttribute(SEMATTRS_HTTP_URL, location.href);
context.with(trace.setSpan(context.active(), fetchSpan), () => {
addSpanNetworkEvents(fetchSpan, entries);
if (!this._getConfig().ignoreNetworkEvents) {
addSpanNetworkEvents(fetchSpan, entries);
}
this._addCustomAttributesOnSpan(
fetchSpan,
this._getConfig().applyCustomAttributesOnSpan?.documentFetch
Expand All @@ -135,21 +137,30 @@ export class DocumentLoadInstrumentation extends InstrumentationBase {

this._addResourcesSpans(rootSpan);

addSpanNetworkEvent(rootSpan, PTN.FETCH_START, entries);
addSpanNetworkEvent(rootSpan, PTN.UNLOAD_EVENT_START, entries);
addSpanNetworkEvent(rootSpan, PTN.UNLOAD_EVENT_END, entries);
addSpanNetworkEvent(rootSpan, PTN.DOM_INTERACTIVE, entries);
addSpanNetworkEvent(
rootSpan,
PTN.DOM_CONTENT_LOADED_EVENT_START,
entries
);
addSpanNetworkEvent(rootSpan, PTN.DOM_CONTENT_LOADED_EVENT_END, entries);
addSpanNetworkEvent(rootSpan, PTN.DOM_COMPLETE, entries);
addSpanNetworkEvent(rootSpan, PTN.LOAD_EVENT_START, entries);
addSpanNetworkEvent(rootSpan, PTN.LOAD_EVENT_END, entries);
if (!this._getConfig().ignoreNetworkEvents) {
addSpanNetworkEvent(rootSpan, PTN.FETCH_START, entries);
addSpanNetworkEvent(rootSpan, PTN.UNLOAD_EVENT_START, entries);
addSpanNetworkEvent(rootSpan, PTN.UNLOAD_EVENT_END, entries);
addSpanNetworkEvent(rootSpan, PTN.DOM_INTERACTIVE, entries);
addSpanNetworkEvent(
rootSpan,
PTN.DOM_CONTENT_LOADED_EVENT_START,
entries
);
addSpanNetworkEvent(
rootSpan,
PTN.DOM_CONTENT_LOADED_EVENT_END,
entries
);
addSpanNetworkEvent(rootSpan, PTN.DOM_COMPLETE, entries);
addSpanNetworkEvent(rootSpan, PTN.LOAD_EVENT_START, entries);
addSpanNetworkEvent(rootSpan, PTN.LOAD_EVENT_END, entries);
}

if (!this._getConfig().ignorePerformancePaintEvents) {
addSpanPerformancePaintEvents(rootSpan);
}
Copy link

Choose a reason for hiding this comment

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

There is a way to do better and avoid calling the Performance API altogether. The function getPerformanceNavigationEntries and in utils.js could return an empty entries map when this flag is turned off.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think doing it this way makes it very explicit in the code where this option is being used which is why I would prefer to do it this way. This function will not get called if ignorePerformanceEventsis set to true so the Performance API won't be called this way either.

Copy link

Choose a reason for hiding this comment

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

Sorry I put this comment in the wrong place. I meant it can be avoided even here - https://github.com/honeycombio/opentelemetry-js-contrib/blob/feat/user-interaction-instrumentation-span-events/plugins/web/opentelemetry-instrumentation-document-load/src/instrumentation.ts#L103

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks like the entries variable is actively used by the _startSpan function to determine the start time so I'm inclined to leave this as is.


addSpanPerformancePaintEvents(rootSpan);
this._addCustomAttributesOnSpan(
rootSpan,
this._getConfig().applyCustomAttributesOnSpan?.documentLoad
Expand Down Expand Up @@ -197,7 +208,9 @@ export class DocumentLoadInstrumentation extends InstrumentationBase {
);
if (span) {
span.setAttribute(SEMATTRS_HTTP_URL, resource.name);
addSpanNetworkEvents(span, resource);
if (!this._getConfig().ignoreNetworkEvents) {
addSpanNetworkEvents(span, resource);
}
this._addCustomAttributesOnResourceSpan(
span,
resource,
Expand Down
34 changes: 34 additions & 0 deletions plugins/web/opentelemetry-instrumentation-document-load/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,38 @@ export interface DocumentLoadInstrumentationConfig
documentFetch?: DocumentLoadCustomAttributeFunction;
resourceFetch?: ResourceFetchCustomAttributeFunction;
};

/** Ignore adding network events as span events for document fetch and resource fetch spans.
* This instrumentation will send the following span events by default:
* connectEnd
* connectStart
* decodedBodySize
* domComplete
* domContentLoadedEventEnd
* domContentLoadedEventStart
* domInteractive
* domainLookupEnd
* domainLookupStart
* encodedBodySize
* fetchStart
* loadEventEnd
* loadEventStart
* navigationStart
* redirectEnd
* redirectStart
* requestStart
* responseEnd
* responseStart
* secureConnectionStart
* unloadEventEnd
* unloadEventStart
*/
ignoreNetworkEvents?: boolean;

/** Ignore adding performance paint span events on document load spans
* This instrumentation will send the following span events by default:
* firstContentfulPaint
* firstPaint
*/
ignorePerformancePaintEvents?: boolean;
}
70 changes: 70 additions & 0 deletions plugins/web/opentelemetry-instrumentation-document-load/test/documentLoad.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -755,6 +755,76 @@ describe('DocumentLoad Instrumentation', () => {
});
});
});

describe('ignore span events if specified', () => {
let spyEntries: any;
beforeEach(() => {
spyEntries = sandbox.stub(window.performance, 'getEntriesByType');
spyEntries.withArgs('navigation').returns([entries]);
spyEntries.withArgs('resource').returns(resources);
spyEntries.withArgs('paint').returns(paintEntries);
});

afterEach(() => {
spyEntries.restore();
});

it('should ignore network span events if ignoreNetworkEvents is set to true', done => {
plugin = new DocumentLoadInstrumentation({
enabled: false,
ignoreNetworkEvents: true,
});
plugin.enable();

setTimeout(() => {
const rootSpan = exporter.getFinishedSpans()[0] as ReadableSpan;
const fetchSpan = exporter.getFinishedSpans()[1] as ReadableSpan;
const loadSpan = exporter.getFinishedSpans()[3] as ReadableSpan;

const rsEvents = rootSpan.events;
const fsEvents = fetchSpan.events;
const lsEvents = loadSpan.events;

assert.strictEqual(exporter.getFinishedSpans().length, 4);
assert.strictEqual(rootSpan.name, 'documentFetch');
assert.strictEqual(rsEvents.length, 0);

assert.strictEqual(fetchSpan.name, 'resourceFetch');
assert.strictEqual(fsEvents.length, 0);

assert.strictEqual(loadSpan.name, 'documentLoad');
assert.deepEqual(
lsEvents.map(event => event.name),
['firstPaint', 'firstContentfulPaint']
);

done();
});
});

it('should ignore performance events if ignorePerformanceEvents is set to true', done => {
plugin = new DocumentLoadInstrumentation({
enabled: false,
ignorePerformancePaintEvents: true,
});
plugin.enable();

setTimeout(() => {
const loadSpan = exporter.getFinishedSpans()[3] as ReadableSpan;
const lsEvents = loadSpan.events;

assert.strictEqual(exporter.getFinishedSpans().length, 4);

assert.strictEqual(loadSpan.name, 'documentLoad');
assert.notInclude(
lsEvents.map(event => event.name),
['firstPaint', 'firstContentfulPaint']
);

done();
});
});
});
});

/**
Expand Down
Loading