[WIP] OTLP Exporter support for http/protobuf #1615
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
robwknox
requested review from
a team,
lzchen and
hectorhdzg
and removed request for
a team
|
Does this conflict with #1604 ? |
owais
reviewed
Mar 11, 2021
| HeadersInput = Union[Dict[str, str], str, None] | ||
| Headers = Dict[str, str] | ||
|
|
||
|
|
There was a problem hiding this comment.
nit: Do any of the following really belong in utils? May be constants.py or symbols.py would be better if not types
| from typing import Dict, Union | ||
|
|
||
| HeadersInput = Union[Dict[str, str], str, None] | ||
| Headers = Dict[str, str] |
There was a problem hiding this comment.
nit: these should be defined either in the module that is consuming them or in a exporter.otlp.types.
|
@robwknox |
|
@lzchen apologies for not dropping a note earlier but I don't have time to continue working on this due to a personal issue that has come up. |
|
@robwknox Closing for now. |
|
Thanks for your help @robwknox! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Adds support for the http/protobuf protocol. Code structure has been refactored to introduce the notions of
Encoders(e.g. Protobuf) andSenders(e.g. GRPC, HTTP) akin to the recent refactoring of the Zipkin Exporter. This was done to decouple the functionality to allow for the introduction of additional implementations of each and, as importantly, for easier and cleaner testing. The structure is also well suited for handling the re-introduction of metric signals when the code becomes stable (I initially had the metric code refactored and included ;) but pulled it out when it was moved to experimental status).Breaking Changes
OTLPSpanExporter:opentelemetry.exporter.otlp.exporter=>opentelemetry.exporter.otlp__init__signature argument changes:protocol: new and requiredcompression: type changed from string to enumheaders: type changed from Sequence to Dict for more apt structure. Headers, whether passed in or retrieved from env vars, are now parsed into a common dict structure before being passed into the Senders.credentials: changed tocertificate_file(optional string). This aligns with the spec and is necessary since http/protobuf can not make use of aChannelCredentialsobject - which is what the type was for credentials.Fixes #1106
Type of change
How Has This Been Tested?
Does This PR Require a Contrib Repo Change?
Checklist: