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 InstrumentationScope and deprecate InstrumentationLibraryInfo #2583

Merged
merged 13 commits into from Apr 21, 2022

Conversation

srikanthccv
Copy link
Member

@srikanthccv srikanthccv commented Apr 2, 2022

Description

This spec PR open-telemetry/opentelemetry-specification#2276 introduced the concept of InstrumentationScope. Naming in our components is varying. The deprecated InstrumentationLibrary from spec is InstrumentationInfo. And the arguments naming have the mixed usage of "module" and "library" instrumenting_module_name and instrumenting_library_version. I introduced another class and properties wherever necessary for backward compatibility and replaced in the experimental packages. I would like to test these changes manually but opening it for reviews.

Fixes #2516

@srikanthccv srikanthccv marked this pull request as ready for review April 5, 2022 10:39
@srikanthccv srikanthccv requested a review from a team as a code owner April 5, 2022 10:39
@ocelotl ocelotl added the Approve Public API check This label shows that the public symbols added or changed in a PR are strictly necessary label Apr 11, 2022
@ocelotl
Copy link
Contributor

ocelotl commented Apr 11, 2022

@srikanthccv please add a changelog ✌️

Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall the change looks good. Would be good to capture the results (maybe just a comment in this PR) from testing this against both the collector v0.47.0 and v0.48.0

Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this change doesnt go out in 1.11.0, the deprecation version will need updating. Please add a changelog entry to let users know of the deprecation

Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please resolve the conflict and check if my suggestions make sense ✌️

@robbkidd
Copy link
Member

Hi! Without the Library -> Scope rename, Honeycomb customers can't send us OTLP traces or metrics with the current OTel Python version (v1.11.0).

Our OTel ingest is pinned to an earlier otlp-proto (0.11.0) to support customers who are sending us metrics from older OTel SDKs using behavior that was deprecated last year. We are unable to arrange for data ingest that supports both that deprecated metric format and this deprecated library/scope rename because the deprecated metric behavior was removed before this rename.

What can we do to help get this rename merged and released?

@srikanthccv
Copy link
Member Author

srikanthccv commented Apr 20, 2022

@robbkidd did you happen to look at this collector issue open-telemetry/opentelemetry-collector#5188?

@srikanthccv srikanthccv merged commit 7647a11 into open-telemetry:main Apr 21, 2022
@srikanthccv srikanthccv deleted the issue-2516 branch April 21, 2022 06:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approve Public API check This label shows that the public symbols added or changed in a PR are strictly necessary
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deprecate InstrumentationLibraryInfo
5 participants