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

feat(sdk-trace-base): add stack trace to operation on ended Span warning #5363

Merged
merged 4 commits into from
Jan 23, 2025

Conversation

neilfordyce
Copy link
Contributor

Which problem is this PR solving?

Fixes #5113

Short description of the changes

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

I tested this manually in a local dev env with some broken instrumentation follows which attempts to add an event after end() is called.

  tracer.startActiveSpan('childSpan', (childSpan) => {
    const promise = createPromise();
    return promise
      .then(() => {
        res.sendStatus(200);
      })
      .catch((err) => {
        childSpan.addEvent('error', err);
      })
      .finally(() => {
        childSpan.end();
        childSpan.addEvent("blaahhhh");   // THIS SHOULD NOT BE ADDED, SHOULD GENERATE WARNING STACKTRACE
      });
  }),

Which results in this warning, with a stack trace attached, pointing straight to the source of the probelm server/routes/index.js:119:19

Cannot execute the operation on ended Span {traceId: fc11e7b2830ee69cf134bf1c639da423, spanId: 0c387f3def287901} Error: Operation attempted on ended Span {traceId: fc11e7b2830ee69cf134bf1c639da423, spanId: 0c387f3def287901}
    at Span._isSpanEnded (/git/tracing/node-opentelemetry/node_modules/@opentelemetry/sdk-trace-base/build/src/Span.js:243:25)
    at Span.addEvent (/git/tracing/node-opentelemetry/node_modules/@opentelemetry/sdk-trace-base/build/src/Span.js:105:18)
    at /git/tracing/node-tracing-reference-app/packages/server/routes/index.js:119:19
    at <anonymous>

Checklist:

  • Followed the style guidelines of this project
  • Unit tests have been added
  • Documentation has been updated

@neilfordyce neilfordyce requested a review from a team as a code owner January 23, 2025 12:41
Victorsesan and others added 3 commits January 23, 2025 12:45
…ded spans, making it easier to trace issues in both Node.js and Browser environments

Reviewer: pichlermarc
Refs: open-telemetry#5113

feat(sdk-trace-base): improved logging mechanism for operations on ended spans, making it easier to trace issues in both Node.js and Browser environments
Reviewer: pichlermarc
Refs: open-telemetry#5113

feat(sdk-trace-base): improved logging mechanism for operations on ended spans, making it easier to trace issues in both Node.js and Browser environments

Reviewer: pichlermarc
Refs: open-telemetry#5113
Copy link

codecov bot commented Jan 23, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.54%. Comparing base (3447582) to head (b484e2e).
Report is 3 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5363   +/-   ##
=======================================
  Coverage   94.54%   94.54%           
=======================================
  Files         318      318           
  Lines        8051     8052    +1     
  Branches     1694     1694           
=======================================
+ Hits         7612     7613    +1     
  Misses        439      439           
Files with missing lines Coverage Δ
packages/opentelemetry-sdk-trace-base/src/Span.ts 97.63% <100.00%> (+0.01%) ⬆️

@neilfordyce neilfordyce changed the title feat(sdk-trace-base): add stacktrace to operation on ended Span warning feat(sdk-trace-base): add stack trace to operation on ended Span warning Jan 23, 2025
Copy link
Member

@pichlermarc pichlermarc left a comment

Choose a reason for hiding this comment

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

Looks good, thank you 🙂

@pichlermarc pichlermarc added this pull request to the merge queue Jan 23, 2025
Merged via the queue into open-telemetry:main with commit 0ae25f1 Jan 23, 2025
15 checks passed
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.

[sdk-trace-base] add debug-level log with stack trace when modifying neded span
3 participants