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 more informative/restrictive typing to Log body field #4124

Closed
lzchen opened this issue Aug 15, 2024 · 6 comments · Fixed by #4185
Closed

Add more informative/restrictive typing to Log body field #4124

lzchen opened this issue Aug 15, 2024 · 6 comments · Fixed by #4185
Labels
bug Something isn't working good first issue Good first issue help wanted logging sdk Affects the SDK package.

Comments

@lzchen
Copy link
Contributor

lzchen commented Aug 15, 2024

Describe your environment

OS: (e.g, Ubuntu)
Python version: (e.g., Python 3.8.10)
SDK version: (e.g., 1.25.0)
API version: (e.g., 1.25.0)

What happened?

From this comment: #4054 (comment)

Change the typing of LogRecord to restrict to the data model definition of Any instead of Python's Any type.

Steps to Reproduce

See notes.

Expected Result

See notes.

Actual Result

See notes.

Additional context

No response

Would you like to implement a fix?

None

@lzchen lzchen added bug Something isn't working good first issue Good first issue sdk Affects the SDK package. help wanted logging labels Aug 15, 2024
@lzchen lzchen mentioned this issue Aug 15, 2024
10 tasks
@wasup-yash
Copy link
Contributor

wasup-yash commented Aug 16, 2024

when #4128 gets merged I would also like to take this up as it is related to it, afaik we just need to make the options take the exporter types, also would like to know if some tests we gonna be writing regarding this ?

@lzchen
Copy link
Contributor Author

lzchen commented Aug 16, 2024

@wasup-yash

Yeah we pretty much want to use a Union of these types instead of the Python any. It will definitely be good to add some tests for this (pass in the various valid/invalid types to try to create a LogRecord).

@Ali-Alnosairi
Copy link
Contributor

Hi @lzchen
can I take this ?

@lzchen
Copy link
Contributor Author

lzchen commented Sep 10, 2024

@Ali-Alnosairi

As @xrmx has mentioned, please choose one issue to work on and open a PR accordingly instead of working on multiple different issues.

@Krishn1412
Copy link

Hey @lzchen, can I pick this one up?

@Ali-Alnosairi
Copy link
Contributor

use a Union of these types

I am about to create a union for the Any type , but I wanted to ask where to add it, is it fine to add it here or create new file !

@lzchen would you please help !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good first issue help wanted logging sdk Affects the SDK package.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants