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

Initial webvitals integration #25

Merged
merged 5 commits into from
Dec 7, 2020
Merged

Initial webvitals integration #25

merged 5 commits into from
Dec 7, 2020

Conversation

johnbley
Copy link
Contributor

@johnbley johnbley commented Dec 2, 2020

No description provided.

src/webvitals.js Outdated
export function initWebVitals(provider) {
const tracer = provider.getTracer('webvitals');
// CLS is defined as being sent more than once, easier to just ensure that everything is sent just on the first occurence.
const reported = {};
Copy link
Contributor

Choose a reason for hiding this comment

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

you can make this var local to this file, because webvitals should be initiated at most once per page

const value = metric.value;
const span = tracer.startSpan('webvitals');
span.setAttribute(name, value);
span.end(span.startTime);
Copy link
Contributor

Choose a reason for hiding this comment

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

have we confirmed this with Justin? or are we deciding now it's going to be sent like that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Excellent point. I'll post on the team channel and seek some input.

Copy link
Contributor

Choose a reason for hiding this comment

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

I was going to approve this, but now I'm not sure, I can't seem to find any conclusions in that Slack thread.

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 read it as "looks good enough" but we can wait another day or two for any more objections/discussion to pop up.

@johnbley johnbley merged commit 7b33aa3 into main Dec 7, 2020
@johnbley johnbley deleted the webvitals branch December 7, 2020 15:15
sfishel-splunk pushed a commit to sfishel-splunk/splunk-otel-js-web that referenced this pull request Jun 22, 2022
Initial webvitals integration
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants