-
Notifications
You must be signed in to change notification settings - Fork 534
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: reduce root span cardinality in express plugin #176
feat: reduce root span cardinality in express plugin #176
Conversation
Signed-off-by: George Gooden <[email protected]>
|
The failures are related to an missing dependency in the user interaction plugin (angular/angular#38526). Should I fix this by pinning the version of the dependency in this PR or should it be addressed another specific fix PR? |
Not sure but i believe #175 solves the issue |
I'm unlinking the linked issue since it only solves it for a single package. |
It looks like the build fails because our plan with circleci no longer supports the machine type we were using. Were we downgraded to a free plan? Perhaps we should consider moving tests fully to github actions as other sigs have done? |
@gecgooden the tests are failing because they are running in your personal CircleCI account. You need to update your account in circleci to follow this repo. |
6a2dd20
to
87f57e6
Compare
Codecov Report
@@ Coverage Diff @@
## master #176 +/- ##
=======================================
Coverage 95.06% 95.06%
=======================================
Files 110 110
Lines 5877 5881 +4
Branches 605 607 +2
=======================================
+ Hits 5587 5591 +4
Misses 290 290
|
Which problem is this PR solving?
Fixes (partiall) open-telemetry/opentelemetry-js#897 for the express plugin. Span names should be low cardinality, and indicate that a span belongs to a class of spans.
Spans generated by incoming requests in the http plugin create spans with names such as
GET /users/123
, where123
is a user identifier.This has a high cardinality but the http plugin doesn't have the context to set a low cardinality name such as
GET /users/:id
.Short description of the changes
When patching the request handler in express, we call
updateName
on the root span with the method of the request (egGET
), and the parameterized route (eg/users/:id
) to rename it.