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

Context API is missing CreateKey functionality #1737

Closed
Oberon00 opened this issue Apr 2, 2021 · 4 comments · Fixed by #1853
Closed

Context API is missing CreateKey functionality #1737

Oberon00 opened this issue Apr 2, 2021 · 4 comments · Fixed by #1853

Comments

@Oberon00
Copy link
Member

Oberon00 commented Apr 2, 2021

opentelemetry-python uses strings as keys for Context. However, the spec is written to expect a special Key object https://github.com/open-telemetry/opentelemetry-specification/blob/v1.1.0/specification/context/context.md#create-a-key

The key name exists for debugging purposes and does not uniquely identify the key. Multiple calls to CreateKey with the same name SHOULD NOT return the same value unless language constraints dictate otherwise.

I don't think "language constraints dictate otherwise" for Python (CreateKey could just return a new Key object every time and compare by identity).

Even then this is only a SHOULD requirement that is violated here, so the current implementation can in theory be made spec conformant just by providing a method def create_key(keyname: str): return keyname.

@lzchen
Copy link
Contributor

lzchen commented Apr 7, 2021

@Oberon00
I understand that this is for spec compliancy, but is there much point in creating a CreateKey method if we are using strings as keys? If there is a strong argument for using objects over strings that would make more sense.

@Oberon00
Copy link
Member Author

Oberon00 commented Apr 8, 2021

The argument given in the spec is

Keys are used to allow cross-cutting concerns to control access to their local state. They are unique such that other libraries which may use the same context cannot accidentally use the same key.

This argument is less important for Python where you cannot easily load the same Context-using library multiple times (which can happen for example in Java with the classloader mechanism).

Another argument would be that object identity comparison might be faster than string comparison.

@euniceek
Copy link
Contributor

Hi, I would like to give this one a try!

@codeboten
Copy link
Contributor

Thanks @eunice98k, all yours!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants