Skip to content

fix: report disabled otel request context#171

Open
0XFF-96 wants to merge 2 commits into
fastify:mainfrom
0XFF-96:fix-opentelemetry-enabled-state
Open

fix: report disabled otel request context#171
0XFF-96 wants to merge 2 commits into
fastify:mainfrom
0XFF-96:fix-opentelemetry-enabled-state

Conversation

@0XFF-96

@0XFF-96 0XFF-96 commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Issue Link: #170

feat: expose instrumented request context state

Core change:

  • restore enabled to application/route-level meaning
  • add instrumented: span != null && context != null
  • use instrumented as the TS discriminator
  • update tests for ignorePaths / disabled / otel false
enabled
= otel is enabled by global/route config

instrumented
= this specific request actually has span/context

Checklist

@Tony133
Tony133 requested a review from metcoder95 July 9, 2026 07:56

@metcoder95 metcoder95 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is sadly not the fix we should apply, the enabled flag will just state weather or not the otel instrumentation is enabled for the route or the application, not if the handler has been instrumented or not.

A suggestion here will be to add another flag that hints wether or not the handler has been instrumented.
e.g. instrumented

@0XFF-96

0XFF-96 commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

This is sadly not the fix we should apply, the enabled flag will just state weather or not the otel instrumentation is enabled for the route or the application, not if the handler has been instrumented or not.

A suggestion here will be to add another flag that hints wether or not the handler has been instrumented. e.g. instrumented

Hi metcoder, thanks for you review.

I will look into it and see where we can solve this by introducing a new parameter or not.

Signed-off-by: Jimmy li <jili449668@gmail.com>
@0XFF-96
0XFF-96 force-pushed the fix-opentelemetry-enabled-state branch from 4a7792c to 0354c24 Compare July 10, 2026 03:32
@0XFF-96

0XFF-96 commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

how the request looks like

stateDiagram-v2
  [*] --> IncomingRequest

  IncomingRequest --> GloballyDisabled: instrumentation.disable()
  IncomingRequest --> RouteDisabled: config.otel = false
  IncomingRequest --> IgnoredPath: ignorePaths matches
  IncomingRequest --> Instrumented: normal request

  GloballyDisabled --> RequestContext
  RouteDisabled --> RequestContext
  IgnoredPath --> RequestContext
  Instrumented --> RequestContext

  RequestContext --> [*]

  GloballyDisabled: enabled = false
  GloballyDisabled: instrumented = false
  GloballyDisabled: span/context = null

  RouteDisabled: enabled = false
  RouteDisabled: instrumented = false
  RouteDisabled: span/context = null

  IgnoredPath: enabled = true
  IgnoredPath: instrumented = false
  IgnoredPath: span/context = null

  Instrumented: enabled = true
  Instrumented: instrumented = true
  Instrumented: span/context = available
Loading

@0XFF-96

0XFF-96 commented Jul 11, 2026

Copy link
Copy Markdown
Contributor Author

Mutation Test report

Targeted mutants: 7
Killed: 5
Survived: 2
Score: 71.4%
Practical score: 100% if treating span-only/context-only as equivalent under current control flow

@0XFF-96
0XFF-96 requested a review from metcoder95 July 11, 2026 00:24
@0XFF-96

This comment was marked as spam.

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