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: include instrumentation scope info in console span and log record exporters #4848

Merged
merged 6 commits into from
Jul 17, 2024

Conversation

blumamir
Copy link
Member

@blumamir blumamir commented Jul 4, 2024

The ConsoleSpanExporter prints many useful information to the console about a span, but it lacks the instrumentationLibrary. This info is present on the ReadableSpan, but it was probably forgotten, as the resource is included and the span info is included, but there is no way currently to view the library info for an output span:

{
  resource: {
    attributes: {
      'service.name': 'coupon',
      'telemetry.sdk.language': 'nodejs',
      'telemetry.sdk.name': 'opentelemetry',
      'telemetry.sdk.version': '1.25.0',
      'telemetry.distro.name': 'odigos',
      'telemetry.distro.version': 'v1.0.76'
    }
  },
  traceId: 'cd873e9e3f16df24a8e2ebfe83d00b9c',
  parentId: '22ca73ebb95febbe',
  traceState: undefined,
  name: 'request handler - /apply-coupon',
  id: 'cb7f45d392356f99',
  kind: 0,
  timestamp: 1720132143829000,
  duration: 15.166,
  attributes: {
    'http.route': '/apply-coupon',
    'express.name': '/apply-coupon',
    'express.type': 'request_handler'
  },
  status: { code: 0 },
  events: [],
  links: []
}

This PR adds this info the the output.

@blumamir blumamir requested a review from a team July 4, 2024 22:39
@@ -65,6 +65,7 @@ export class ConsoleSpanExporter implements SpanExporter {
resource: {
attributes: span.resource.attributes,
},
instrumentationLibrary: span.instrumentationLibrary,
Copy link
Contributor

Choose a reason for hiding this comment

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

Aside: Do you think we'll ever change the property to span.instrumentationScope given that the InstrumentationLibrary type is deprecated (https://github.com/open-telemetry/opentelemetry-js/blob/main/packages/opentelemetry-core/src/common/types.ts#L42-L63) ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we should emit it with the name instrumentationScope. Updated the PR Thanks 👍

CHANGELOG.md Outdated Show resolved Hide resolved
@trentm
Copy link
Contributor

trentm commented Jul 4, 2024

Lint failures.

@blumamir blumamir changed the title feat: print instrumentation library info with console span exporter feat: print instrumentation scope info to console span and log record exporter Jul 13, 2024
@blumamir blumamir changed the title feat: print instrumentation scope info to console span and log record exporter feat: include instrumentation scope info in console span and log record exporters Jul 13, 2024
Copy link

codecov bot commented Jul 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.24%. Comparing base (ecc88a3) to head (6eec63e).
Report is 55 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4848      +/-   ##
==========================================
+ Coverage   91.04%   93.24%   +2.20%     
==========================================
  Files          89      337     +248     
  Lines        1954     9622    +7668     
  Branches      416     2020    +1604     
==========================================
+ Hits         1779     8972    +7193     
- Misses        175      650     +475     
Files Coverage Δ
...es/sdk-logs/src/export/ConsoleLogRecordExporter.ts 87.50% <ø> (ø)
...y-sdk-trace-base/src/export/ConsoleSpanExporter.ts 82.35% <ø> (ø)

... and 256 files with indirect coverage changes

@blumamir
Copy link
Member Author

Thank you @trentm for the review, addressed your good suggestions and fixed the lint 👍

Copy link
Member

@hectorhdzg hectorhdzg left a comment

Choose a reason for hiding this comment

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

LGTM

@trentm trentm added this pull request to the merge queue Jul 17, 2024
Merged via the queue into open-telemetry:main with commit 8d88c36 Jul 17, 2024
21 checks passed
Zirak pushed a commit to Zirak/opentelemetry-js that referenced this pull request Sep 14, 2024
…rd exporters (open-telemetry#4848)

* feat: print instrumentation library info with console span exporter

* chore: add changelog

* test: fix tests for console span exporter

* feat: instrumentation scope for both logs and spans

* chore: update changelog
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.

4 participants