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

Proposal for pure syntactic sugar event API #2917

Closed
jack-berg opened this issue Nov 3, 2022 · 7 comments · Fixed by #2941
Closed

Proposal for pure syntactic sugar event API #2917

jack-berg opened this issue Nov 3, 2022 · 7 comments · Fixed by #2941
Assignees
Labels
area:api Cross language API specification issue spec:logs Related to the specification/logs directory

Comments

@jack-berg
Copy link
Member

Proposal to change the event API to pure syntactic sugar over the log API.

Currently, the event API shows up in two places in the log API:

  1. LoggerProvider - Get A Logger: event_domain is an optional field, required if you intend to use the resulting Logger to emit events.
  2. Logger - Emit Event. Emit a named event with the event_domain specified when initializing the Logger.

Alternatively, the event API could just be a wrapper around Logger:

// Get a LoggerProvider
LoggerProvider loggerProvider = GlobalLoggerProvider.get();

// Create a EventLogger, which is configured with a Logger and a domain to be added as event.domain
// The EventLogger will delegate to the Logger when building / emitting events
EventLogger eventLogger = EventLogger.create(loggerProvider.get("logger-name"), "my-event-domain");

// Build / emit an event
eventLogger.eventBuilder("my-event-name")
    .setAttribute(AttributeKey.stringKey("k1"), "v1")
    .setAttribute(AttributeKey.stringKey("k2"), "v2")
    .emit();
// Emits event with attributes {"event.domain": "my-event-domain", "event.name": "my-event-name", "k1": "v1", "k2": "v2"}

Sample code here. Sample source for EventLogger.

In this design, EventLogger has a concrete implementation rather than being an interface. As a result, there's no ability to have alternative implementations. Can still provide alternative implementations for Logger, which EventLogger delegates to.

Advantages include:

  • The log API is decoupled from the event API. The Event API depends on log API, not the other way around. This makes it somewhat easier to work towards stabilizing logs independently of events. (However, I do think its possible to have mixed stability with the logs API where Emit LogRecord is marked as stable while Emit Event is still experimental. Its up to language sigs to handle this how they see fit.)
  • The API better reflects the data model. Its clear that events are just a particular type of log. You can look the source for EventLogger and see plainly that it just delegates to Logger and sets some attributes.

I'm not sure there's any existing places where OpenTelemetry provides syntactic sugar like this. The need for such a thing may be unique to the log signal, since logs are a really raw / low level signal upon which other things may be build (i.e. events, and potentially profiles). Its worth thinking about how additional pseudo-signals built on top of logs might manifest if this pattern is adopted and extended to other use cases.

@jack-berg jack-berg added the spec:logs Related to the specification/logs directory label Nov 3, 2022
@jkwatson
Copy link
Contributor

jkwatson commented Nov 4, 2022

I might argue that this is actually "semantic sugar" rather than "syntactic sugar", but perhaps I've been consuming too much "pedantic sugar" for my own good.

@tigrannajaryan
Copy link
Member

@jack-berg I suggest we split the events "helper/sugar" API into a separate document to simplify status tracking (avoid Mixed status) and to emphasize the difference and reliance on the logging part.

@tigrannajaryan
Copy link
Member

I also still see confusion from people that we building a logging API. We need to somehow make it more clear that we don't.

One thing we can try after splitting the events part is to rename the "Log API" to "Logging Backend API". The term "backend" appears in new designs (e.g. here) and makes it distinct from "frontend". This distinction perfectly matches what we want to do, we are really building a backend, while refraining from building a frontend.

@yurishkuro
Copy link
Member

Maybe SPI instead of API, since it's not intended for application code.

@tigrannajaryan
Copy link
Member

I opened an issue to make sure we fix this confusion: #2936

@tigrannajaryan
Copy link
Member

Maybe SPI instead of API, since it's not intended for application code.

I think the approach that @jack-berg proposed in this PR requires that the LoggerProvider/Logger is callable from application code. EventLogger.create accepts an instance of Logger and EventLogger.create() call lives in the application's (or instrumentation library's) code.

@scheler
Copy link
Contributor

scheler commented Nov 10, 2022

I'm not sure there's any existing places where OpenTelemetry provides syntactic sugar like this. The need for such a thing may be unique to the log signal, since logs are a really raw / low level signal upon which other things may be build (i.e. events, and potentially profiles). Its worth thinking about how additional pseudo-signals built on top of logs might manifest if this pattern is adopted and extended to other use cases.

Just making an observation. Some Splunk libraries have already built an Events API using 0-duration spans. We are choosing to represent Events using LogRecord data model as we feel it's a better fit. The LogRecord data model is not so low level and has several optional fields not relevant to the psuedo-signals built on top, but Spans have a duration making it less suitable for representing Events than LogRecords.

tigrannajaryan pushed a commit that referenced this issue Nov 14, 2022
carlosalberto pushed a commit to carlosalberto/opentelemetry-specification that referenced this issue Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:api Cross language API specification issue spec:logs Related to the specification/logs directory
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants