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

Clarify OTLP/JSON behavior when receiving unknown message fields (not enum fields) #425

Closed
Tracked by #1957
bogdandrutu opened this issue Aug 17, 2022 · 1 comment · Fixed by open-telemetry/opentelemetry-specification#2816
Assignees

Comments

@bogdandrutu
Copy link
Member

Usually protobuf tries to preserve the unknown fields, and works ok if we have otlp/proto (http/grpc) receiver -> otlp/proto (http/proto) exporter, aka passthrough mode. This works by keeping the bytes received untouched and put them back on the wire.

When converting between two different encodings OTLP/JSON to OTLP/Proto, or OTLP/Proto to Jaeger these fields cannot be converted.

Question is, what should we do when receiving unknown fields:

  1. Fail to decode and reject the entire message? This is the default behavior of jsonpb (JSON protobuf) decoder, probably too aggressive.
  2. Drop the data to ensure consistent behavior independent of the "export" encoding, to ensure that this does not work even in the case of passthrough mode (consistent with other modes).
  3. Keep the data, works okish for protobuf encoding (especially if data stored or passed around as protobuf), but does not really work well for JSON, or if try to store/pass as proto after.
@tigrannajaryan
Copy link
Member

I believe that in the spirit of our interoperability approach we should go with option 2. This allows adding new fields in the future and such fields should be ignored by receivers who don't know about the new fields.

Option 3 is a possible option for services that need to passthrough the data (such as Otel Collector). However, I do not think we need to make this part of the OTLP spec. It is a specialized application and does not need to be in the spec.

@tigrannajaryan tigrannajaryan self-assigned this Sep 13, 2022
@tigrannajaryan tigrannajaryan changed the title Clarify protocol behavior when receiving unknown message fields (not enum fields) Clarify OTLP/JSON behavior when receiving unknown message fields (not enum fields) Sep 13, 2022
tigrannajaryan added a commit to tigrannajaryan/opentelemetry-specification that referenced this issue Sep 20, 2022
Resolves open-telemetry/opentelemetry-proto#425

The proposed behavior is necessary for interoperability of senders and receivers
when OTLP protocol evolves in an allowed way: by adding new fields to existing
messages.
tigrannajaryan added a commit to tigrannajaryan/opentelemetry-specification that referenced this issue Sep 20, 2022
Resolves open-telemetry/opentelemetry-proto#425

The proposed behavior is necessary for interoperability of senders and receivers
when OTLP protocol evolves in an allowed way: by adding new fields to existing
messages.
tigrannajaryan added a commit to tigrannajaryan/opentelemetry-specification that referenced this issue Sep 20, 2022
Resolves open-telemetry/opentelemetry-proto#425

The proposed behavior is necessary for interoperability of senders and receivers
when OTLP protocol evolves in an allowed way: by adding new fields to existing
messages.
tigrannajaryan added a commit to tigrannajaryan/opentelemetry-specification that referenced this issue Sep 20, 2022
Resolves open-telemetry/opentelemetry-proto#425

The proposed behavior is necessary for interoperability of senders and receivers
when OTLP protocol evolves in an allowed way: by adding new fields to existing
messages.
tigrannajaryan added a commit to tigrannajaryan/opentelemetry-specification that referenced this issue Sep 20, 2022
Resolves open-telemetry/opentelemetry-proto#425

The proposed behavior is necessary for interoperability of senders and receivers
when OTLP protocol evolves in an allowed way: by adding new fields to existing
messages.
tigrannajaryan added a commit to tigrannajaryan/opentelemetry-specification that referenced this issue Sep 20, 2022
Resolves open-telemetry/opentelemetry-proto#425

The proposed behavior is necessary for interoperability of senders and receivers
when OTLP protocol evolves in an allowed way: by adding new fields to existing
messages.
tigrannajaryan added a commit to tigrannajaryan/opentelemetry-specification that referenced this issue Sep 22, 2022
Resolves open-telemetry/opentelemetry-proto#425

The proposed behavior is necessary for interoperability of senders and receivers
when OTLP protocol evolves in an allowed way: by adding new fields to existing
messages.
tigrannajaryan added a commit to tigrannajaryan/opentelemetry-specification that referenced this issue Sep 26, 2022
Resolves open-telemetry/opentelemetry-proto#425

The proposed behavior is necessary for interoperability of senders and receivers
when OTLP protocol evolves in an allowed way: by adding new fields to existing
messages.
yurishkuro added a commit to open-telemetry/opentelemetry-specification that referenced this issue Sep 27, 2022
…2816)

* Clarify that unknown fields must be ignored when receiving OTLP/JSON

Resolves open-telemetry/opentelemetry-proto#425

The proposed behavior is necessary for interoperability of senders and receivers
when OTLP protocol evolves in an allowed way: by adding new fields to existing
messages.

* Remove unnecessary sentence

* Fix typo

Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
ChengJinbao added a commit to ChengJinbao/opentelemetry-specification that referenced this issue Nov 16, 2022
…(#2816)

* Clarify that unknown fields must be ignored when receiving OTLP/JSON

Resolves open-telemetry/opentelemetry-proto#425

The proposed behavior is necessary for interoperability of senders and receivers
when OTLP protocol evolves in an allowed way: by adding new fields to existing
messages.

* Remove unnecessary sentence

* Fix typo

Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
tigrannajaryan added a commit to tigrannajaryan/opentelemetry-proto that referenced this issue Apr 20, 2023
…(#2816)

* Clarify that unknown fields must be ignored when receiving OTLP/JSON

Resolves open-telemetry#425

The proposed behavior is necessary for interoperability of senders and receivers
when OTLP protocol evolves in an allowed way: by adding new fields to existing
messages.

* Remove unnecessary sentence

* Fix typo

Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
tigrannajaryan added a commit to tigrannajaryan/opentelemetry-proto that referenced this issue Apr 21, 2023
…(#2816)

* Clarify that unknown fields must be ignored when receiving OTLP/JSON

Resolves open-telemetry#425

The proposed behavior is necessary for interoperability of senders and receivers
when OTLP protocol evolves in an allowed way: by adding new fields to existing
messages.

* Remove unnecessary sentence

* Fix typo

Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
tigrannajaryan added a commit that referenced this issue Apr 27, 2023
…(#2816)

* Clarify that unknown fields must be ignored when receiving OTLP/JSON

Resolves #425

The proposed behavior is necessary for interoperability of senders and receivers
when OTLP protocol evolves in an allowed way: by adding new fields to existing
messages.

* Remove unnecessary sentence

* Fix typo

Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
joaopgrassi pushed a commit to dynatrace-oss-contrib/semantic-conventions that referenced this issue Mar 21, 2024
…(#2816)

* Clarify that unknown fields must be ignored when receiving OTLP/JSON

Resolves open-telemetry/opentelemetry-proto#425

The proposed behavior is necessary for interoperability of senders and receivers
when OTLP protocol evolves in an allowed way: by adding new fields to existing
messages.

* Remove unnecessary sentence

* Fix typo

Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants