Rename InstrumentationLibrary to InstrumentationScope

[tigrannajaryan]

This applies changes described in spec PR open-telemetry/opentelemetry-specification#2276

This is not a breaking change for OTLP. The wire format does not change.

The change is done in a way that the transition can be handled gracefully for OTLP/JSON and for the generated code. See the comments for explanation on how to do it in the senders and receivers.

[alolita]

Naming changes are hard. They should be considered as breaking changes for any developers (as an end-user). We should be treating what we call "stable" as "stable". Going forward, can we treat the names of fields and types are unbreakable (i.e. stable). We need to stabilize message, fields, enum names.

Imo, having a name change as in this PR should not be happening at this stage of the project.

Re: https://github.com/open-telemetry/opentelemetry-proto/blob/main/README.md

Let's discuss in the spec SIG tomorrow.

[tigrannajaryan]

Naming changes are hard. They should be considered as breaking changes for any developers (as an end-user). We should be treating what we call "stable" as "stable".

I agree. We do not call names "stable" though. They are clearly and explicitly specified to be unstable.

Going forward, can we treat the names of fields and types are unbreakable (i.e. stable). We need to stabilize message, fields, enum names.

I agree and I think we are very close to achieving this. Logs proto are about to be declared stable, races and metrics already are. With all 3 signals stable we should be more confident to say that we are done making breaking changes, including the names, etc.

[MSNev]

Adding comment from the Spec SIG.
I believe we should be versioning the spec rather than double publishing / consuming as double publishing / consuming introduces a bunch of unnecessary complexity and is going to introduce more code to handle "duplicated" fields.

And I agree with @alolita comment

Imo, having a name change as in this PR should not be happening at this stage of the project.

[tigrannajaryan]

We discussed this in the spec SIG and we believe that stability guarantees need to be extended to cover the JSON format and the Protobuf message and field names. This will require an examination of the current names and JSON format to make sure everything is named as we want it to be and then we can extend the guarantees. This can be done after the batch of changes proposed by this PR is done (and this PR may be re-worked to make the transition more graceful).

[tigrannajaryan]

Adding comment from the Spec SIG. I believe we should be versioning the spec rather than double publishing / consuming as double publishing / consuming introduces a bunch of unnecessary complexity and is going to introduce more code to handle "duplicated" fields.

Versioning like that is extremely undesirable because it means breaking interoperability between versions. We will not do that. We promised wire compatibility for binary Protobuf format and in the past we publicly recognized that breaking interoperability between network nodes is one of the most painful form of causing user trouble, we should avoid that, even at the high cost of pains to us, developers. Versioning like that may be the last resort, which is not the case right now, we have more graceful options.

And I agree with @alolita comment

Imo, having a name change as in this PR should not be happening at this stage of the project.

I agree it is not undesirable, so going forward we need to rethink our guarantees.

[tigrannajaryan]

All, I made changes to the PR to make the transition more graceful. Please read the comments here and let me know what you think.

[dyladan]

The compromise seems acceptable to me. So JSON senders should send only the new name by default but offer an old collector compatibility option which sends both?

[tigrannajaryan]

The compromise seems acceptable to me. So JSON senders should send only the new name by default but offer an old collector compatibility option which sends both?

Yes, that's what I was aiming for, but open for alternate suggestions.

In the Collector we will also implement gracefully accepting either the old or the new name, so it should work fine with old or new senders.

Essentially we are ensuring interoperability from both sides. If at least one of the sides correctly implements the new recommendations it should be able to work with an old peer.

[tigrannajaryan]

All, I made changes to the PR to make the transition more graceful. Please read the comments here and let me know what you think.

@MadVikingGod @Aneurysm9 what do you think?

[Aneurysm9]

I think this is a good way to transition. Do we also need to do the same for ResourceMetrics?

[anuraaga]

The compromise seems acceptable to me. So JSON senders should send only the new name by default but offer an old collector compatibility option which sends both?

For reference, I suspect we probably wouldn't update the Java JSON exporter to send both old and new as JSON export from Java should be unpopular enough that gating users on updating their collector is fine IMO.

If we were to send both in JSON, we would send both in binary too though as deciding the behavior for one or the other would complicate our export code quite a bit.

[tigrannajaryan]

I think this is a good way to transition. Do we also need to do the same for ResourceMetrics?

Yes, I will add that if we are happy with this approach.

[tigrannajaryan]

The compromise seems acceptable to me. So JSON senders should send only the new name by default but offer an old collector compatibility option which sends both?

For reference, I suspect we probably wouldn't update the Java JSON exporter to send both old and new as JSON export from Java should be unpopular enough that gating users on updating their collector is fine IMO.

If we were to send both in JSON, we would send both in binary too though as deciding the behavior for one or the other would complicate our export code quite a bit.

I think this is OK. The proposed approach exceeds the formal guarantees that we have to follow and is not an absolute requirement (I deliberately used "SHOULD" clauses instead of "MUST"). If the particular SIG feels JSON is not an important case for them then they just need to follow the formal requirements of the "Alpha" stability, which says "you can break it anytime".

For the record: Collector doesn't even have an implementation of OTLP/JSON exporter, so nothing to do there. The Collector has an OTLP/JSON receiver, which we should be able to modify to accept both old and new formats gracefully.

[tigrannajaryan]

I think this is a good way to transition. Do we also need to do the same for ResourceMetrics?

Done now for metrics and logs too.

[tigrannajaryan]

@open-telemetry/specs-approvers please review.

[bogdandrutu]

For me this looks good.

[tigrannajaryan]

@open-telemetry/specs-trace-approvers please review.

[tigrannajaryan]

@open-telemetry/specs-logs-approvers please review.

[tigrannajaryan]

We have enough approvals. @dyladan @MadVikingGod @Aneurysm9 I will merge unless you object.

[MadVikingGod]

I have not had a chance to actually confirm that this won't be a breaking change. That said I do think that this should work. I don't want to hold this up any further.