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

OTLP exporters should emit standard user agent string #2958

Closed
codeboten opened this issue Oct 4, 2022 · 8 comments · Fixed by #3009
Closed

OTLP exporters should emit standard user agent string #2958

codeboten opened this issue Oct 4, 2022 · 8 comments · Fixed by #3009

Comments

@codeboten
Copy link
Contributor

As per the following change in the specification (1.14 releasing Oct 4):

OpenTelemetry protocol exporters SHOULD emit a User-Agent header to at a minimum identify the exporter, the language of its implementation, and the version of the exporter. For example, the Python OTLP exporter version 1.2.3 would report the following:
OTel OTLP Exporter Python/1.2.3

See https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/exporter.md#user-agent

codeboten pushed a commit to codeboten/opentelemetry-python that referenced this issue Oct 5, 2022
Adding user agent string to OTLP HTTP exporter. As part of the change, I refactored the content-type header as well.

Part of open-telemetry#2958
ocelotl pushed a commit that referenced this issue Oct 12, 2022
* `exporter-otlp-proto-http`: add user agent string

Adding user agent string to OTLP HTTP exporter. As part of the change, I refactored the content-type header as well.

Part of #2958

* update changelog

* fix linting, fix link

Co-authored-by: Srikanth Chekuri <[email protected]>
@pridhi-arora
Copy link
Contributor

Hi! I would like to take this as part of my outreachy contribution?
@srikanthccv Can you explain to me what part is left, as discussed on Slack? Thanks in advance.

@srikanthccv
Copy link
Member

@pridhi-arora The last mentioned pull request only added the user agent header for the HTTP exporter https://github.com/open-telemetry/opentelemetry-python/tree/main/exporter/opentelemetry-exporter-otlp-proto-http but the same should also be done for the gRPC exporter https://github.com/open-telemetry/opentelemetry-python/tree/main/exporter/opentelemetry-exporter-otlp-proto-grpc. More specifically this self._headers created here should have the user agent included.

@pridhi-arora
Copy link
Contributor

Thanks, @srikanthccv. I will have something by tomorrow.

@pridhi-arora
Copy link
Contributor

@srikanthccv
I have limited knowledge of protobuffs and grpc. Reading while working on this.
I wanted to understand what do we place instead of the following?
https://github.com/open-telemetry/opentelemetry-python/pull/2959/files#diff-0a0769c6ffd79e4f93558febb4d8575ddc9d9acdc1d4c3d0dc2c6328f832658dR77

_OTLP_HTTP_HEADERS = {
    "Content-Type": "application/x-protobuf",
    "User-Agent": "OTel OTLP Exporter Python/" + __version__,
}

From what I got to know by the way we send data in headers for grpc is

metadata = (('md-key', 'some value'),
   ('some-md-key', 'another value'))

Keeping this format in mind. Does the following sound good?

_OTLP_GRPC_HEADERS = (('Content-Type', 'application/x-protobuf'),
   ('User-Agent', 'OTel OTLP Exporter Python/' + __versio))

@srikanthccv
Copy link
Member

Content-Type is just for HTTP exporter and was part of some minor refactoring. You need not do that in gRPC exporter. And yes the User-Agent should be tuple.

@pridhi-arora
Copy link
Contributor

pridhi-arora commented Nov 1, 2022

@srikanthccv How do I update the header in _log_exporter/init.py
From HTTP, I see that I can do it as self._session.headers.update(_OTLP_HTTP_HEADERS). But, how do I do it here?

@srikanthccv
Copy link
Member

If you notice, it calls super().__init__ i.e., OTLPExporterMixin so you only have to change that class.

@pridhi-arora
Copy link
Contributor

@srikanthccv I have tried something here. Can you please take a look?
#3009

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants