Fix OTLP/JSON to use lowerCamelCase keys

[tigrannajaryan]

Once the change open-telemetry/opentelemetry-specification#2829 is released (likely in spec 1.15.0) we can update the OTLP implementations here to stop allowing original field names and only accept lowerCamelCase.

[bogdandrutu]

this is not a bug correct?

[tigrannajaryan]

I think being non-compliant with a spec is a bug :-)

[bogdandrutu]

I would not remove that for few months, since people may have already deployed apps that send the original proto names.

Do you think that is not the case?

[tigrannajaryan]

I would not remove that for few months, since people may have already deployed apps that send the original proto names.

Do you think that is not the case?

I agree. Let's keep for now and remove in a few months.

[tigrannajaryan]

Note a bug discovered in the spec: open-telemetry/opentelemetry-proto#476

I would suggest to wait for resolution of that issue before we change anything in the Collector.

[mx-psi]

@tigrannajaryan open-telemetry/opentelemetry-proto/issues/476 was resolved, and this still seems to be an issue:

opentelemetry-collector/pdata/pmetric/json.go

Line 208 in df4ba7d

case "timeUnixNano", "time_unix_nano":

Did the proto issue change anything about what we want to do here?

Also, do we consider this a bug? Is this allowed after 1.0?

[mx-psi]

From the November 15th SIG meeting, there are two questions here:

1. Do we actually want to remove this?
   • This does not impose a performance penalty
   • We don't claim pdata to be strictly compliant with the OTLP/JSON spec
   • But the OTLP receiver must be compliant with the spec
2. If we do, is this a breaking change?
   • Changing this would not change the type signature of any Go symbol
   • It would change the behavior, it is unclear under what circumstances a change in behavior is a breaking change (e.g. we can change it for bugs)

[tigrannajaryan]

Copying the list from the Chosen Solution here:

1. We recognize this a bug that needs to be fixed.
2. We choose camelCase as canonical JSON field casing. Use correct lowerCameCase for JSON field references opentelemetry-proto#468 fixed it in the spec.
3. We make sure Collector's OTLP receiver accepts both capitalizations for one year to give everyone time to transition.
4. All SDK exporters and Collector's OTLP exporter will be immediately updated to use the camelCase (if not already).

Items 3 and 4 are the one's actionable for the Collector. Number 4 needs to be acted asap (if not already) and number 3 we can do on May 9, 2024 (one year after the OTLP spec PR was merged).

[tigrannajaryan]

Do we actually want to remove this?

I think I would prefer to remove it. It is code that needs to be maintained, it is not spec compliant and is not necessary. I feel that one year is enough grace period to deprecate it.

If we do, is this a breaking change?

Not in the formal sense. We are simply making the implementation compliant with the spec.

[mx-psi]

We are simply making the implementation compliant with the spec.

Okay, this is convincing to me, I filed #8974 to make it crystal clear that our JSONMarshaler and JSONUnmarshaler implementations follow the OTLP/JSON spec.

IMO this clears out the way for pdata 1.0 :)

[atoulme]

Can we close this issue at this time?

[mx-psi]

@atoulme No, we need to fix this by dropping support for lowerCamelCase on May next year.