-
Notifications
You must be signed in to change notification settings - Fork 652
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
[WIP] user-facing Logs API that includes events #4213
base: main
Are you sure you want to change the base?
Conversation
38acfc3
to
87c2b08
Compare
opentelemetry-api/src/opentelemetry/_logs/_internal/__init__.py
Outdated
Show resolved
Hide resolved
@abstractmethod | ||
def emit( | ||
self, | ||
name: str = None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the spec missing the name parameter
https://opentelemetry.io/docs/specs/otel/logs/api/#emit-a-logrecord ? BTW in the sdk you are using event_name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's just got merged - open-telemetry/opentelemetry-specification#4320 will be released soon
BTW in the sdk you are using event_name
Good catch!
23b1945
to
3c09ed0
Compare
when you feel ready, can you put a diff of the old vs new into the PR desc or make a self-comment on a line that shows the high level change. It is a little harder to guess which part of this diff are the important end user parts, and I'd love to tell give feedback on the presumably better experience. |
Moves emit-event (with a bit of convenience) into Logs API and makes it user-facing, see open-telemetry/oteps#265 and open-telemetry/opentelemetry-specification#4236 for the context.
This is an early prototype to drive discussions, high-level API feedback is very welcome.