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

fix(fastify): Use plugin name for middleware span name #1680

Merged
merged 3 commits into from
Sep 26, 2023

Conversation

mydea
Copy link
Contributor

@mydea mydea commented Sep 12, 2023

I noticed that I was getting a bunch middleware - anonymous spans from my fastify instrumentation.

I think it would be more useful to see e.g. middleware - router or middleware - fastify -> @fastify/multipart -> @fastify/helmet there?

@mydea mydea requested a review from a team September 12, 2023 15:19
@codecov
Copy link

codecov bot commented Sep 12, 2023

Codecov Report

Merging #1680 (eb778ba) into main (47301c0) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #1680   +/-   ##
=======================================
  Coverage   91.36%   91.36%           
=======================================
  Files         129      129           
  Lines        6402     6402           
  Branches     1281     1281           
=======================================
  Hits         5849     5849           
  Misses        553      553           
Files Changed Coverage
...try-instrumentation-fastify/src/instrumentation.ts 100.00%

@mydea mydea force-pushed the fn/fastify-middleware-name branch 2 times, most recently from 5b315e4 to b1c64a0 Compare September 13, 2023 07:54
@mydea mydea force-pushed the fn/fastify-middleware-name branch from b1c64a0 to 061da0c Compare September 13, 2023 08:12
Copy link
Member

@blumamir blumamir left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you for the contribution

@blumamir blumamir merged commit 4503d3e into open-telemetry:main Sep 26, 2023
@dyladan dyladan mentioned this pull request Sep 26, 2023
mydea added a commit to getsentry/sentry-javascript that referenced this pull request Oct 11, 2023
…9223)

E2E tests started failing for fastify because of 0.32.3
(https://github.com/open-telemetry/opentelemetry-js-contrib/blob/main/plugins/node/opentelemetry-instrumentation-fastify/CHANGELOG.md)
being released. This includes this change
open-telemetry/opentelemetry-js-contrib#1680
(which actually we wrote xD) that changes some span names, which lead to
E2E test failing.
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.

3 participants