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

Duplicate instrument handling in API #2142

Closed
Tracked by #2140
aabmass opened this issue Sep 23, 2021 · 4 comments · Fixed by #2409
Closed
Tracked by #2140

Duplicate instrument handling in API #2142

aabmass opened this issue Sep 23, 2021 · 4 comments · Fixed by #2409
Assignees
Labels
1.10.0rc1 release candidate 1 for metrics GA api Affects the API package. metrics

Comments

@aabmass
Copy link
Member

aabmass commented Sep 23, 2021

  • Should it be handled at the global level?
  • Which instrument fields are considered non-identifying (description?)
@aabmass aabmass added the api Affects the API package. label Dec 14, 2021
@ocelotl
Copy link
Contributor

ocelotl commented Dec 15, 2021

  • Should it be handled at the global level?
  • Which instrument fields are considered non-identifying (description?)

I am a bit confused:

  1. What should be handled at the global level?
  2. @aabmass, is your intention to suggest description is a non-identifying instrument field?

@lzchen lzchen added the 1.10.0rc1 release candidate 1 for metrics GA label Jan 11, 2022
@lzchen
Copy link
Contributor

lzchen commented Jan 19, 2022

From specs:

Meter implementations MUST return an error when multiple Instruments are registered under the same Meter instance using the same name.

So should probably be done as part of error handling.

@ocelotl
Copy link
Contributor

ocelotl commented Jan 25, 2022

From specs:

Meter implementations MUST return an error when multiple Instruments are registered under the same Meter instance using the same name.

So should probably be done as part of error handling.

Should this issue be closed @lzchen in favor of implementing this later with general error handling?

@lzchen
Copy link
Contributor

lzchen commented Jan 25, 2022

@ocelotl
I think the behavior won't completely be fixed with error handling. We still need to implement the "checking of valid instrument creation" part.

@ocelotl ocelotl self-assigned this Jan 25, 2022
ocelotl added a commit to ocelotl/opentelemetry-python that referenced this issue Jan 26, 2022
ocelotl added a commit to ocelotl/opentelemetry-python that referenced this issue Jan 27, 2022
ocelotl added a commit to ocelotl/opentelemetry-python that referenced this issue Feb 23, 2022
ocelotl added a commit to ocelotl/opentelemetry-python that referenced this issue Mar 3, 2022
ocelotl added a commit to ocelotl/opentelemetry-python that referenced this issue Mar 7, 2022
ocelotl added a commit to ocelotl/opentelemetry-python that referenced this issue Mar 25, 2022
ocelotl added a commit to ocelotl/opentelemetry-python that referenced this issue Apr 12, 2022
ocelotl added a commit that referenced this issue Apr 12, 2022
* Add check for duplicate instrument names

Fixes #2142

* Add repeated instrument names for SDK as well

* Update opentelemetry-api/src/opentelemetry/_metrics/__init__.py

Co-authored-by: Srikanth Chekuri <[email protected]>

* Add instrument names lock

* Replace raised exception for a warning

* Added some fiexes

* Add double check

* Add type annotations

* Update opentelemetry-api/src/opentelemetry/_metrics/__init__.py

Co-authored-by: Srikanth Chekuri <[email protected]>

* Move check invocation to SDK

* Fix lint

* Separate instrument checking from warning logging

* Use consistent class

* Fix typo in example (#2597)

* Fix docs

Co-authored-by: Srikanth Chekuri <[email protected]>
Co-authored-by: Alex Boten <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.10.0rc1 release candidate 1 for metrics GA api Affects the API package. metrics
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants