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 meter handling in SDK #2141

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

Duplicate meter handling in SDK #2141

aabmass opened this issue Sep 23, 2021 · 5 comments · Fixed by #2373
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 we return the same meter or new meter when get_meter() is called with the same params?

@marjoripomarole
Copy link

I am experimenting with the alpha release of the library on a personal project, exporting to Google Cloud Monitoring. I think it would be more intuitive for the API to return the same meter. Conceptually, it just seems to me that the meter acts like a global object and I don't actually need more than one copy of it to create the multiple counters I have in different files.

@aabmass
Copy link
Member Author

aabmass commented Sep 27, 2021

I agree, I think that approach is also solves the problem of duplicate instruments (#2142)

@ocelotl
Copy link
Contributor

ocelotl commented Sep 28, 2021

Ok, I think we can implement as @mpomarole suggests, the spec allows it IIUC:

It is unspecified whether or under which conditions the same or different Meter instances are returned from this functions.

@lzchen
Copy link
Contributor

lzchen commented Nov 12, 2021

@aabmass

I agree, I think that approach is also solves the problem of duplicate instruments

Isn't there still the issue for identifying what denotes an instrument as unique? For example, with the same meter, if we were to create two counters with the same name, unit and description(?)?

@lzchen lzchen self-assigned this Nov 12, 2021
@aabmass aabmass added the api Affects the API package. label Dec 14, 2021
@lzchen lzchen added the 1.10.0rc1 release candidate 1 for metrics GA label Jan 11, 2022
@lzchen lzchen changed the title Duplicate meter handling in API Duplicate meter handling in SDK Jan 13, 2022
@lzchen
Copy link
Contributor

lzchen commented Jan 13, 2022

Changing this to be an implementation in the SDK instead of the API. This behavior is not defined in the API spec and seems to be a specific implementation detail. It will also be much cleaner (since InstrumentationInfo is in the sdk) and we don't have to deal with proxies in the api.

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.

4 participants