This repository has been archived by the owner on May 3, 2024. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 87
feat(tracing): add OpenTelemetry tracing #1214
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Size Change: +353 B (0%) Total Size: 719 kB
ℹ️ View Unchanged
|
10xLaCroixDrinker
force-pushed
the
feature/tracing
branch
from
December 21, 2023 16:41
7a5f1e3
to
12f14eb
Compare
10xLaCroixDrinker
force-pushed
the
feature/tracing
branch
from
January 16, 2024 21:13
64ae7c0
to
4436375
Compare
…o feature/tracing
…o feature/tracing
…o feature/tracing
10xLaCroixDrinker
force-pushed
the
feature/tracing
branch
from
February 1, 2024 00:44
1feeab3
to
a298c2c
Compare
… outbound requests
JAdshead
reviewed
Mar 6, 2024
Co-authored-by: Jonny Adshead <JAdshead@users.noreply.github.com>
JAdshead
previously approved these changes
Mar 7, 2024
…o match level "info"
JAdshead
previously approved these changes
Mar 8, 2024
Matthew-Mallimo
suggested changes
Mar 11, 2024
giulianok
reviewed
Mar 11, 2024
giulianok
reviewed
Mar 11, 2024
giulianok
reviewed
Mar 11, 2024
giulianok
suggested changes
Mar 11, 2024
get setAttributes() { return () => noopSpan; }, | ||
get setStatus() { return () => noopSpan; }, | ||
get spanContext() { return () => {}; }, | ||
get updateName() { return () => noopSpan; }, |
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.
why the use of get
if set
isn't use? I don't see the value of it
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.
That is the point. Using a getter only is a common pattern. This object is reused over and over again, and this helps prevent changes.
Matthew-Mallimo
approved these changes
Mar 11, 2024
code-forger
approved these changes
Mar 12, 2024
dogpatch626
approved these changes
Mar 12, 2024
giulianok
approved these changes
Mar 12, 2024
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Adds OTel tracing to v6.
The tracer initialization needs to be required via node's
--require
flag for instrumentation to work properly. In order to facilitate this with the least burden to the user a new startup script was added to use this flag when theOTEL_EXPORTER_OTLP_TRACES_ENDPOINT
env var is set.The fastify automatic instrumentation was very noisy, adding a span to every plugin. If anyone thinks this should still be included, I'm open to a discussion, but it didn't seem to provide enough value to outweigh the cost/DX of sheer number of spans added.
Automatic HTTP and DNS tracing is included.
The manually created spans match those in the v5 tracer. I added
phase
attributes to the spans to match the spans in the v5 tracer. Need to have a brief discussion on whether these attributes should be left in.Fixed a bug where ending a request timer more than once would cause a crash.
Motivation and Context
Improves DX, observability & monitoring. Once this is in we will be able to begin work on a new API allowing modules to add their own spans.
How Has This Been Tested?
Performance results
High Volume simple SSR
Test configuration
K6 results with tracing
K6 results without tracing
Grafana visualization
without tracing on the left, with tracing on the rightLow Volume complex SSR
Test configuration
K6 results with tracing
K6 results without tracing
Grafana visualization
with tracing on the left, without tracing on the rightTypes of Changes
Checklist: