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

feat(honeycomb-opentelemetry-web): Add data attributtes for LCP. #309

Merged
merged 8 commits into from
Oct 9, 2024
Merged

feat(honeycomb-opentelemetry-web): Add data attributtes for LCP. #309

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ca84821
Add data attributtes for LCP.
wolfgangcodes Sep 26, 2024
56a9c13
Update test.
wolfgangcodes Sep 26, 2024
04dc434
Update example.
wolfgangcodes Sep 26, 2024
8ed947c
Add docs, tests.
wolfgangcodes Sep 27, 2024
b1c7867
noop
wolfgangcodes Sep 30, 2024
c3f6422
Use dataset API.
wolfgangcodes Oct 8, 2024
ae0ce75
Update docs.
wolfgangcodes Oct 9, 2024
217f708
Update docs.
wolfgangcodes Oct 9, 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
1 change: 1 addition & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ Pass these options to the HoneycombWebSDK:
| enabled | optional | boolean | `true` | Where or not to enable this auto instrumentation. |
| lcp| optional| VitalOpts | `undefined` | Pass-through config options for web-vitals. See [ReportOpts](https://github.com/GoogleChrome/web-vitals?tab=readme-ov-file#reportopts).
| lcp.applyCustomAttributes| optional| function | `undefined` | A function for adding custom attributes to core web vitals spans.
| lcp.dataAttributes| optional| `string[]` | `undefined` | an array of data-* attribute names to filter. By default it will send all `data-*` attribute-value pairs with the key `lcp.element.data-someAttr`, `[]` will send none.
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is how the values end up being added as attrs, according to the code each one is an attribute by itself with .data. prefix, can we change that here?

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 wanted to make sure we were 👍 with c3f6422 before updating all the docs. 😅

| cls| optional| VitalOpts | `undefined` | Pass-through config options for web-vitals. See [ReportOpts](https://github.com/GoogleChrome/web-vitals?tab=readme-ov-file#reportopts).
| cls.applyCustomAttributes| optional| function | `undefined` | A function for adding custom attributes to core web vitals spans.
| inp| optional| VitalOptsWithTimings | `undefined` | Pass-through config options for web-vitals. See [ReportOpts](https://github.com/GoogleChrome/web-vitals?tab=readme-ov-file#reportopts).
Expand Down
38 changes: 19 additions & 19 deletions packages/honeycomb-opentelemetry-web/examples/hello-world-web/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,23 +1,23 @@
<!DOCTYPE html>
<!doctype html>
<html lang="en">
<head>
<meta charset="utf-8" />
<meta name="viewport" content="width=device-width, initial-scale=1" />
<title>Honeycomb OpenTelemetry Web Distro</title>
</head>
<body>
<head>
<meta charset="utf-8" />
<meta name="viewport" content="width=device-width, initial-scale=1" />
<title>Honeycomb OpenTelemetry Web Distro</title>
</head>
<body>
<ul id="spans-go-here"></ul>
<section class="example-app">
<header class="header">
<h1>👋 Hello World</h1>
</header>
<section class="example-app">
<header class="header">
<h1 data-hello="hello" data-foo="42" data-bar-biz>👋 Hello World</h1>
</header>

<button id="loadDadJoke">Get A Random Dad Joke</button>
<div>
<span id="dadJokeText"></span>
</div>
</section>
<!-- Scripts here. Don't remove ↓ -->
<script type="module" src="build/bundle.js"></script>
</body>
<button id="loadDadJoke">Get A Random Dad Joke</button>
<div>
<span id="dadJokeText"></span>
</div>
</section>
<!-- Scripts here. Don't remove ↓ -->
<script type="module" src="build/bundle.js"></script>
</body>
</html>
3 changes: 3 additions & 0 deletions packages/honeycomb-opentelemetry-web/examples/hello-world-web/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ const main = () => {
contextManager: new ZoneContextManager(),
webVitalsInstrumentationConfig: {
vitalsToTrack: ['CLS', 'FCP', 'FID', 'INP', 'LCP', 'TTFB'],
lcp: {
dataAttributes: ['hello', 'barBiz'],
},
},
});
sdk.start();
Expand Down
3 changes: 2 additions & 1 deletion packages/honeycomb-opentelemetry-web/examples/hello-world-web/package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

98 changes: 63 additions & 35 deletions packages/honeycomb-opentelemetry-web/src/web-vitals-autoinstrumentation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,19 @@ interface VitalOpts extends ReportOpts {
applyCustomAttributes?: ApplyCustomAttributesFn;
}

interface VitalOptsWithTimings extends VitalOpts {
interface LcpVitalOpts extends VitalOpts {
/**
* Will filter the values of these data attributes if provided, otherwise will send all data-* attributes an LCP entry
* An empty allow list, such as { dataAttributes: [] } will disable sending data-* attributes
*/
dataAttributes?: string[];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This pattern could be useful if we wanted to add in the ability to collect data- from nodes where attributes aren't available and we need to do a dom lookup. Maybe this is a way to let users opt-into something that might impact "performance"?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmmmmm that's in interesting point, I agree that its nice to have this array if you only want to look up a certain subset of data- attributes. I was originally thinking that if any data- attrs are provided that we would just automatically send them all as attributes and then the array can filter them down if provided just to save the user a setup step but if this is more future proof than I'm happy with it!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ooo. default to sending all and filter down? I like that better for this instance.

}

interface InpVitalOpts extends VitalOpts {
/**
* if this is true it will create spans from the PerformanceLongAnimationFrameTiming frames
*/
includeTimingsAsSpans: boolean;
includeTimingsAsSpans?: boolean;
}

// To avoid importing InstrumentationAbstract from:
Expand Down Expand Up @@ -210,13 +218,13 @@ export interface WebVitalsInstrumentationConfig extends InstrumentationConfig {
vitalsToTrack?: Array<Metric['name']>;

/** Config specific to LCP (Largest Contentful Paint) */
lcp?: VitalOpts;
lcp?: LcpVitalOpts;

/** Config specific to CLS (Cumulative Layout Shift) */
cls?: VitalOpts;

/** Config specific to INP (Interaction to Next Paint) */
inp?: VitalOptsWithTimings;
inp?: InpVitalOpts;

/** Config specific to FID (First Input Delay) */
fid?: VitalOpts;
Expand All @@ -235,9 +243,9 @@ export interface WebVitalsInstrumentationConfig extends InstrumentationConfig {
*/
export class WebVitalsInstrumentation extends InstrumentationAbstract {
readonly vitalsToTrack: Array<Metric['name']>;
readonly lcpOpts?: VitalOpts;
readonly lcpOpts?: LcpVitalOpts;
readonly clsOpts?: VitalOpts;
readonly inpOpts?: VitalOptsWithTimings;
readonly inpOpts?: InpVitalOpts;
readonly fidOpts?: VitalOpts;
readonly fcpOpts?: VitalOpts;
readonly ttfbOpts?: VitalOpts;
Expand Down Expand Up @@ -280,41 +288,37 @@ export class WebVitalsInstrumentation extends InstrumentationAbstract {
private _setupWebVitalsCallbacks() {
if (this.vitalsToTrack.includes('CLS')) {
onCLS((vital) => {
this.onReportCLS(vital, this.clsOpts?.applyCustomAttributes);
this.onReportCLS(vital, this.clsOpts);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Update all the this.onREportXYZ to take opts as the second argument, to minimize changes for additional options.

}, this.clsOpts);
}

if (this.vitalsToTrack.includes('LCP')) {
onLCP((vital) => {
this.onReportLCP(vital, this.lcpOpts?.applyCustomAttributes);
this.onReportLCP(vital, this.lcpOpts);
}, this.lcpOpts);
}

if (this.vitalsToTrack.includes('INP')) {
onINP((vital) => {
this.onReportINP(
vital,
this.inpOpts?.applyCustomAttributes,
this.inpOpts?.includeTimingsAsSpans,
);
this.onReportINP(vital, this.inpOpts);
}, this.inpOpts);
}

if (this.vitalsToTrack.includes('FID')) {
onFID((vital) => {
this.onReportFID(vital, this.fidOpts?.applyCustomAttributes);
this.onReportFID(vital, this.fidOpts);
}, this.fidOpts);
}

if (this.vitalsToTrack.includes('TTFB')) {
onTTFB((vital) => {
this.onReportTTFB(vital, this.ttfbOpts?.applyCustomAttributes);
this.onReportTTFB(vital, this.ttfbOpts);
}, this.ttfbOpts);
}

if (this.vitalsToTrack.includes('FCP')) {
onFCP((vital) => {
this.onReportFCP(vital, this.fcpOpts?.applyCustomAttributes);
this.onReportFCP(vital, this.fcpOpts);
}, this.fcpOpts);
}
}
Expand Down Expand Up @@ -417,10 +421,8 @@ export class WebVitalsInstrumentation extends InstrumentationAbstract {
});
}

onReportCLS = (
cls: CLSMetricWithAttribution,
applyCustomAttributes?: ApplyCustomAttributesFn,
) => {
onReportCLS = (cls: CLSMetricWithAttribution, clsOpts: VitalOpts = {}) => {
const { applyCustomAttributes } = clsOpts;
if (!this.isEnabled()) return;

const { name, attribution } = cls;
Expand Down Expand Up @@ -451,10 +453,8 @@ export class WebVitalsInstrumentation extends InstrumentationAbstract {
span.end();
};

onReportLCP = (
lcp: LCPMetricWithAttribution,
applyCustomAttributes?: ApplyCustomAttributesFn,
) => {
onReportLCP = (lcp: LCPMetricWithAttribution, lcpOpts: LcpVitalOpts = {}) => {
const { applyCustomAttributes, dataAttributes } = lcpOpts;
if (!this.isEnabled()) return;

const { name, attribution } = lcp;
Expand All @@ -465,6 +465,7 @@ export class WebVitalsInstrumentation extends InstrumentationAbstract {
resourceLoadDelay,
resourceLoadDuration,
elementRenderDelay,
lcpEntry,
}: LCPAttribution = attribution;
const attrPrefix = this.getAttrPrefix(name);

Expand All @@ -481,6 +482,36 @@ export class WebVitalsInstrumentation extends InstrumentationAbstract {
[`${attrPrefix}.resource_load_time`]: resourceLoadDuration,
});

const el: HTMLElement = lcpEntry?.element as HTMLElement;
if (el.dataset) {
for (const attrName in el.dataset) {
const attrValue = el.dataset[attrName];
if (
// Value exists (including the empty string AND either
attrValue !== undefined &&
// dataAttributes is undefined (i.e. send all values as span attributes) OR
(dataAttributes === undefined ||
// dataAttributes is specified AND attrName is in dataAttributes (i.e attribute name is in the supplied allowList)
(dataAttributes && attrName in dataAttributes))
) {
span.setAttribute(
`${attrPrefix}.element.data.${attrName}`,
attrValue,
);
}
}
}
if (dataAttributes)
dataAttributes?.forEach((attrName) => {
const attrValue = el.dataset[attrName];
if (attrValue !== undefined) {
span.setAttribute(
`${attrPrefix}.element.data.${attrName}`,
attrValue,
);
}
});

if (applyCustomAttributes) {
applyCustomAttributes(lcp, span);
}
Expand All @@ -490,9 +521,9 @@ export class WebVitalsInstrumentation extends InstrumentationAbstract {

onReportINP = (
inp: INPMetricWithAttribution,
applyCustomAttributes?: ApplyCustomAttributesFn,
includeTimingsAsSpans = false,
inpOpts: InpVitalOpts = { includeTimingsAsSpans: false },
) => {
const { applyCustomAttributes, includeTimingsAsSpans } = inpOpts;
if (!this.isEnabled()) return;

const { name, attribution } = inp;
Expand Down Expand Up @@ -550,10 +581,8 @@ export class WebVitalsInstrumentation extends InstrumentationAbstract {
);
};

onReportFCP = (
fcp: FCPMetricWithAttribution,
applyCustomAttributes?: ApplyCustomAttributesFn,
) => {
onReportFCP = (fcp: FCPMetricWithAttribution, fcpOpts: VitalOpts = {}) => {
const { applyCustomAttributes } = fcpOpts;
if (!this.isEnabled()) return;

const { name, attribution } = fcp;
Expand All @@ -580,10 +609,8 @@ export class WebVitalsInstrumentation extends InstrumentationAbstract {
/**
* @deprecated this will be removed in the next major version, use INP instead.
*/
onReportFID = (
fid: FIDMetricWithAttribution,
applyCustomAttributes?: ApplyCustomAttributesFn,
) => {
onReportFID = (fid: FIDMetricWithAttribution, fidOpts: VitalOpts = {}) => {
const { applyCustomAttributes } = fidOpts;
if (!this.isEnabled()) return;

const { name, attribution } = fid;
Expand All @@ -608,8 +635,9 @@ export class WebVitalsInstrumentation extends InstrumentationAbstract {

onReportTTFB = (
ttfb: TTFBMetricWithAttribution,
applyCustomAttributes?: ApplyCustomAttributesFn,
ttfbOpts: VitalOpts = {},
) => {
const { applyCustomAttributes } = ttfbOpts;
if (!this.isEnabled()) return;

const { name, attribution } = ttfb;
Expand Down
Loading
Loading