-
Notifications
You must be signed in to change notification settings - Fork 806
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
chore: set default service name #2227
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2227 +/- ##
==========================================
+ Coverage 92.23% 92.29% +0.05%
==========================================
Files 143 144 +1
Lines 5154 5168 +14
Branches 1060 1064 +4
==========================================
+ Hits 4754 4770 +16
+ Misses 400 398 -2
|
packages/opentelemetry-resources/src/platform/node/service-name.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
overall lgtm, will approve when the default service name problem has a solution
Should have just read spec. It states what the fallback should be if not provided. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what about jaeger exporter ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add example to readme upgrade guidelines how to define service name for collector exporter now. Other than that lgtm
Since the 0.20.0 release and the pull request open-telemetry#2227 it seems that serviceName is deprecated and we should use service.name instead. I havn't easily found this information in the changelog, it did require some digging to understand why the service name was now a very long unknown_service:/path/to/node. Documentation says use service.name Resource attribute instead. but it wasn't about replacing serviceName: 'yourServiceName' by service: { name: 'yourServiceName'}.
This is an alternative to #2154 and fixes #2143