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

Event basics #265

Merged
merged 27 commits into from
Sep 19, 2024
Merged

Event basics #265

merged 27 commits into from
Sep 19, 2024

Conversation

trask
Copy link
Member

@trask trask commented Aug 27, 2024

No description provided.

text/9999-event-vision.md Outdated Show resolved Hide resolved
text/9999-event-vision.md Outdated Show resolved Hide resolved
Copy link
Member

@jack-berg jack-berg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this is just a draft, but I support the ideas here. Let's have the debate about the high level direction of events and logs once, here. Assuming we agree and merge, we can focus on the details of making the vision a reality, without having to re-litigate the overall direction with each discussion.

text/0265-event-vision.md Outdated Show resolved Hide resolved
text/0265-event-vision.md Outdated Show resolved Hide resolved
text/0265-event-vision.md Outdated Show resolved Hide resolved
text/0265-event-vision.md Outdated Show resolved Hide resolved
text/0265-event-vision.md Outdated Show resolved Hide resolved
@trask
Copy link
Member Author

trask commented Aug 29, 2024

(just a heads up to reviewers, due to volume of comments, I'm resolving those that I think are addressed so we can focus on the remaining questions, but please feel free to unresolve any if you think they need more discussion)

EDIT: since some of the discussions are quite long, instead of unresolving it may help instead to start a new discussion with a summary of remaining discussion points (along with a pointer to the prior discussion for full context)

text/0265-event-vision.md Outdated Show resolved Hide resolved
text/0265-event-vision.md Outdated Show resolved Hide resolved
text/0265-event-vision.md Outdated Show resolved Hide resolved
text/0265-event-vision.md Outdated Show resolved Hide resolved
text/0265-event-vision.md Outdated Show resolved Hide resolved
text/0265-event-vision.md Outdated Show resolved Hide resolved
text/0265-event-vision.md Outdated Show resolved Hide resolved
text/0265-event-vision.md Outdated Show resolved Hide resolved
@lmolkova
Copy link

lmolkova commented Aug 30, 2024

WRT why do we need new APIs (leaving naming aside), I created this comparison of key features

Feature Event/Logging v2 API 'Traditional' Logging API
Formatted string body ❔ not a priority ✅ common case
Complex body ✅ common case ❌ usually not supported or stringified
Extra properties ✅ attributes ✅ attributes (or not supported)
Named and identifiable ✅ always ❔ (depends on app, inconsistent)
Predictable structure ✅ highly encouraged ❔ (depends on app, inconsistent)
Semantics and stability guarantees ➕ depends on app, encouraged ❔ (depends on app, usually no expectations)
How to consume 💻 query, aggregate, auto-analyze ❔ read + grep by humans, parse and re-create structure to be used by tools. Depends on app
Filtering by scope + name + severity (API support) future ✅ yes, usually by scope + severity
Severity ❔ optional ✅ required
Context ✅ explicit ❔ implicit or none

[update] big thanks to @cijothomas for pointing out difference around severity

Copy link
Member

@dyladan dyladan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Thanks for putting this together

Copy link
Member

@pellared pellared left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

Currently, we have Logs Bridge API and Logs SDK.

Shouldn't we rename Log API to Logs API in this document?

text/0265-event-vision.md Outdated Show resolved Hide resolved
Copy link
Member

@svrnm svrnm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@trask trask requested a review from MSNev September 13, 2024 17:13
Copy link
Contributor

@scheler scheler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Events API can be merged with Logs API as long as the Logs API does not evolve to become bloated with a boat load of features that other logging libraries provide. Those will not be applicable to Events, so someone wanting just Events API should not be forced to use the much-larger Logs API.

text/0265-event-vision.md Outdated Show resolved Hide resolved
Copy link
Member

@XSAM XSAM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍 I like this!

text/0265-event-vision.md Outdated Show resolved Hide resolved
@jsuereth
Copy link
Contributor

This OTEP now passes requirements to merge, outside of waiting 2 business days after the last change. This will be merged tomorrow.

@jsuereth jsuereth merged commit 87e944b into open-telemetry:main Sep 19, 2024
4 checks passed
@trask trask deleted the event-basics branch September 19, 2024 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.