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

Add bytes (binary) as a data type to AnyValue #297

Merged

Conversation

tigrannajaryan
Copy link
Member

Contributes to:
open-telemetry/opentelemetry-specification#780
open-telemetry/opentelemetry-collector-contrib#3267

Issue open-telemetry/opentelemetry-specification#780
nicely describes why the data type is needed. There are several use cases
for binary data, both for trace and log attributes and for log record Body.

This is a backward compatible addition. After this change is merged no senders
will initially exist that emit binary data. Nevertheless, if such data is received
by the Collector it will correctly pass such data intact through the pipeline
when receiving/sending OTLP (no Collector code changes are needed for this).

We do not yet have binary data type in the OpenTelemetry API, so no existing
sources can emit it yet.

The receivers that do not understand the binary data type should also continue
functioning normally. Collector's current implementation treats any unknown
data type as NULL (and this would apply to binary data type until we teach
the Collector to understand binary specifically). I checked the Collector source
code and this should not result in crashes or overly surprising behavior
(NULL is either ignored or treated as an "unknown" branch in the code which
does not care about it).

We will add full support for binary data to the Collector, particularly to
support translating it correctly to other formats (e.g. Jaeger, which supports
binary type natively).

Note: the addition of this data type to the protocol is not an obligation to
expose the data type in the Attributes API. OTLP has always been a superset of
what is possible to express via the API. The addition of the data type in the
Attributes API should be discussed separately in the specification repo.

Contributes to:
open-telemetry/opentelemetry-specification#780
open-telemetry/opentelemetry-collector-contrib#3267

Issue open-telemetry/opentelemetry-specification#780
nicely describes why the data type is needed. There are several use cases
for binary data, both for trace and log attributes and for log record Body.

This is a backward compatible addition. After this change is merged no senders
will initially exist that emit binary data. Nevertheless, if such data is received
by the Collector it will correctly pass such data intact through the pipeline
when receiving/sending OTLP (no Collector code changes are needed for this).

We do not yet have binary data type in the OpenTelemetry API, so no existing
sources can emit it yet.

The receivers that do not understand the binary data type should also continue
functioning normally. Collector's current implementation treats any unknown
data type as NULL (and this would apply to binary data type until we teach
the Collector to understand binary specifically). I checked the Collector source
code and this should not result in crashes or overly surprising behavior
(NULL is either ignored or treated as an "unknown" branch in the code which
does not care about it).

We will add full support for binary data to the Collector, particularly to
support translating it correctly to other formats (e.g. Jaeger, which supports
binary type natively).

Note: the addition of this data type to the protocol is not an obligation to
expose the data type in the Attributes API. OTLP has always been a superset of
what is possible to express via the API. The addition of the data type in the
Attributes API should be discussed separately in the specification repo.
@tigrannajaryan tigrannajaryan requested a review from a team April 28, 2021 18:54
@tigrannajaryan
Copy link
Member Author

@open-telemetry/specs-approvers it would be good to have a few more eyes on this before I merge it.

@tigrannajaryan tigrannajaryan merged commit 871ec9c into open-telemetry:main May 5, 2021
@tigrannajaryan tigrannajaryan deleted the feature/tigran/add-binary branch May 5, 2021 18:46
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.

6 participants