-
Notifications
You must be signed in to change notification settings - Fork 437
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
Add user agent for OTLP http/grpc client #1657
Add user agent for OTLP http/grpc client #1657
Conversation
Signed-off-by: owent <[email protected]>
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.
LGTM
Thanks for the fix :)
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.
Thanks for the quick turn around on the fix! Will there be a separate PR to add the user agent to the grpc exporter or does this address both http and grpc OTLP exporters?
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.
LGTM
exporters/otlp/include/opentelemetry/exporters/otlp/otlp_http_client.h
Outdated
Show resolved
Hide resolved
This is for http OTLP exporter. Separate PR would be done for gRPC. |
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.
LGTM. Thanks for adding this.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #1657 +/- ##
==========================================
+ Coverage 84.39% 84.41% +0.02%
==========================================
Files 170 170
Lines 5130 5130
==========================================
+ Hits 4329 4330 +1
+ Misses 801 800 -1
|
Signed-off-by: owent <[email protected]>
BTW, I hope gRPC-cpp allows overriding default user-agent using |
Signed-off-by: owent <[email protected]>
gRPC user agent is added now. We could use |
Should we use |
Good point. I think we should rename, as produce-token can't contain spaces. |
exporters/otlp/include/opentelemetry/exporters/otlp/otlp_environment.h
Outdated
Show resolved
Hide resolved
@codeboten - What do you think of the format without spaces, to be in-compliant with the rfc? |
As discussed in the SIG meeting, we will stick to the user-agent format as given in the specs , and change it later if required. Merging the PR based on this. |
Signed-off-by: owent [email protected]
Fixes #1656
Changes
Add user agent for OTLP http/grpc client
For significant contributions please make sure you have completed the following items:
CHANGELOG.md
updated for non-trivial changes