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] Decide if original and lowerCamelCase JSON keys should be supported #2795

Closed
Tracked by #1957
tigrannajaryan opened this issue Sep 13, 2022 · 6 comments · Fixed by #2829
Closed
Tracked by #1957

[otlp] Decide if original and lowerCamelCase JSON keys should be supported #2795

tigrannajaryan opened this issue Sep 13, 2022 · 6 comments · Fixed by #2829
Assignees
Labels
spec:protocol Related to the specification/protocol directory triaged-accepted The issue is triaged and accepted by the OTel community, one can proceed with creating a PR proposal

Comments

@tigrannajaryan
Copy link
Member

tigrannajaryan commented Sep 13, 2022

The decision that we need to make is whether both the original proto field name and the camelCase version of the field name should be acceptable. It is acceptable by Prototobuf JSON definition. Do we see any need to deviate from this?

@tigrannajaryan
Copy link
Member Author

It is not entirely clear what the definition of JSON-friendliness is. They keys used by OTLP are valid in JSON.

Perhaps one decision that we need to make is whether both the original proto field name and the camelCase version of the field name should be acceptable. It is acceptable by Prototobuf JSON definition. Do we see any need to deviate from this?

@tigrannajaryan
Copy link
Member Author

cc @mwear @dyladan @bogdandrutu @jmacd @Aneurysm9 in case you have any thoughts about this.

@dyladan
Copy link
Member

dyladan commented Sep 13, 2022

I think the default behavior of the protobuf client is to change keys to lowerCamelCase and is likely what the JS exporter is currently doing. I don't see any reason not to accept both though since it is default proto behavior.

@rbailey7210 rbailey7210 added the triaged-accepted The issue is triaged and accepted by the OTel community, one can proceed with creating a PR proposal label Sep 16, 2022
@tigrannajaryan
Copy link
Member Author

I agree with @dyladan that we should just keep the default Protobuf behavior for exporters and also keep the ability for receivers to accept both.

@open-telemetry/specs-approvers any opinions on this?

@dyladan
Copy link
Member

dyladan commented Sep 16, 2022

I don't see any reason to restrict the exporters to this behavior. I would just say receivers should expect both and let exporters do what makes sense for their users expectations.

@tigrannajaryan
Copy link
Member Author

I don't see any reason to restrict the exporters to this behavior. I would just say receivers should expect both and let exporters do what makes sense for their users expectations.

Yes, that's essentially what will happen if we say receivers must accept both.

tigrannajaryan added a commit to tigrannajaryan/opentelemetry-specification that referenced this issue Sep 23, 2022
…OTLP/JSON

Resolves open-telemetry#2795

This is already the current understanding. This is just a clarification and
describes the behavior explicitly.
@tigrannajaryan tigrannajaryan changed the title Verify that current keys are JSON friendly. [otlp] Verify that current keys are JSON friendly. Sep 23, 2022
@tigrannajaryan tigrannajaryan added the spec:protocol Related to the specification/protocol directory label Sep 23, 2022
tigrannajaryan added a commit to tigrannajaryan/opentelemetry-specification that referenced this issue Sep 23, 2022
…OTLP/JSON

Resolves open-telemetry#2795

This is already the current understanding. This is just a clarification and
describes the behavior explicitly.
tigrannajaryan added a commit to tigrannajaryan/opentelemetry-specification that referenced this issue Sep 23, 2022
…OTLP/JSON

Resolves open-telemetry#2795

This is already the current understanding. This is just a clarification and
describes the behavior explicitly.
@tigrannajaryan tigrannajaryan changed the title [otlp] Verify that current keys are JSON friendly. [otlp] Decide if original and lowerCamelCase JSON keys should be supported Sep 27, 2022
tigrannajaryan added a commit to tigrannajaryan/opentelemetry-specification that referenced this issue Oct 5, 2022
Resolves open-telemetry#2795

This is a breaking change for OTLP/JSON and is allowed because OTLP/JSON is not yet Stable.
tigrannajaryan added a commit to tigrannajaryan/opentelemetry-specification that referenced this issue Oct 5, 2022
Resolves open-telemetry#2795

This is a breaking change for OTLP/JSON and is allowed because OTLP/JSON is not yet Stable.
tigrannajaryan added a commit to tigrannajaryan/opentelemetry-specification that referenced this issue Oct 5, 2022
Resolves open-telemetry#2795

This is a breaking change for OTLP/JSON and is allowed because OTLP/JSON is not yet Stable.
tigrannajaryan added a commit to tigrannajaryan/opentelemetry-specification that referenced this issue Oct 12, 2022
Resolves open-telemetry#2795

This is a breaking change for OTLP/JSON and is allowed because OTLP/JSON is not yet Stable.
tigrannajaryan added a commit that referenced this issue Oct 12, 2022
…2829)

Resolves #2795

This is a breaking change for OTLP/JSON and is allowed because OTLP/JSON is not yet Stable.
ChengJinbao added a commit to ChengJinbao/opentelemetry-specification that referenced this issue Nov 16, 2022
…2829)

Resolves open-telemetry/opentelemetry-specification#2795

This is a breaking change for OTLP/JSON and is allowed because OTLP/JSON is not yet Stable.
tigrannajaryan added a commit to tigrannajaryan/opentelemetry-proto that referenced this issue Apr 20, 2023
…2829)

Resolves open-telemetry/opentelemetry-specification#2795

This is a breaking change for OTLP/JSON and is allowed because OTLP/JSON is not yet Stable.
tigrannajaryan added a commit to tigrannajaryan/opentelemetry-proto that referenced this issue Apr 21, 2023
…2829)

Resolves open-telemetry/opentelemetry-specification#2795

This is a breaking change for OTLP/JSON and is allowed because OTLP/JSON is not yet Stable.
tigrannajaryan added a commit to open-telemetry/opentelemetry-proto that referenced this issue Apr 27, 2023
…2829)

Resolves open-telemetry/opentelemetry-specification#2795

This is a breaking change for OTLP/JSON and is allowed because OTLP/JSON is not yet Stable.
joaopgrassi pushed a commit to dynatrace-oss-contrib/semantic-conventions that referenced this issue Mar 21, 2024
…2829)

Resolves open-telemetry/opentelemetry-specification#2795

This is a breaking change for OTLP/JSON and is allowed because OTLP/JSON is not yet Stable.
VinozzZ pushed a commit to VinozzZ/opentelemetry-proto that referenced this issue Jun 21, 2024
…2829)

Resolves open-telemetry/opentelemetry-specification#2795

This is a breaking change for OTLP/JSON and is allowed because OTLP/JSON is not yet Stable.
VinozzZ pushed a commit to VinozzZ/opentelemetry-proto that referenced this issue Jun 21, 2024
…2829)

Resolves open-telemetry/opentelemetry-specification#2795

This is a breaking change for OTLP/JSON and is allowed because OTLP/JSON is not yet Stable.
VinozzZ pushed a commit to VinozzZ/opentelemetry-proto that referenced this issue Jun 21, 2024
…2829)

Resolves open-telemetry/opentelemetry-specification#2795

This is a breaking change for OTLP/JSON and is allowed because OTLP/JSON is not yet Stable.
VinozzZ pushed a commit to VinozzZ/opentelemetry-proto that referenced this issue Jun 21, 2024
…2829)

Resolves open-telemetry/opentelemetry-specification#2795

This is a breaking change for OTLP/JSON and is allowed because OTLP/JSON is not yet Stable.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spec:protocol Related to the specification/protocol directory triaged-accepted The issue is triaged and accepted by the OTel community, one can proceed with creating a PR proposal
Projects
None yet
4 participants