Skip to content

Just riffing to see what this _might_ look like. #18

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.

Sign up for GitHub

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

Closed
breedx-splk wants to merge 1 commit into from
Closed

Just riffing to see what this _might_ look like. #18

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions api/incubator/src/main/java/io/opentelemetry/api/incubator/events/DefaultEventLogger.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ static EventLogger getInstance() {
return INSTANCE;
}

@Override
public <T> void emit(String eventName, AnyValue<T> payload, Attributes attributes){
// noop
}

@Override
public EventBuilder builder(String eventName) {
return NoOpEventBuilder.INSTANCE;
Expand Down Expand Up @@ -63,5 +68,10 @@ public EventBuilder setAttributes(Attributes attributes) {

@Override
public void emit() {}

@Override
public <T> EventBuilder setPayload(AnyValue<T> payload) {
return this;
}
}
}
2 changes: 2 additions & 0 deletions api/incubator/src/main/java/io/opentelemetry/api/incubator/events/EventBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,4 +108,6 @@ default EventBuilder put(String key, boolean... value) {

/** Emit an event. */
void emit();

<T> EventBuilder setPayload(AnyValue<T> payload);
}
5 changes: 5 additions & 0 deletions api/incubator/src/main/java/io/opentelemetry/api/incubator/events/EventLogger.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@

package io.opentelemetry.api.incubator.events;

import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.api.incubator.logs.AnyValue;
import javax.annotation.concurrent.ThreadSafe;

/**
Expand All @@ -31,6 +33,9 @@
@ThreadSafe
public interface EventLogger {

<T> void emit(String eventName, AnyValue<T> payload, Attributes attributes);


/**
* Return a {@link EventBuilder} to emit an event.
*
Expand Down
7 changes: 7 additions & 0 deletions sdk/logs/src/main/java/io/opentelemetry/sdk/logs/internal/SdkEventBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,13 @@ class SdkEventBuilder implements EventBuilder {
this.eventName = eventName;
}

@Override
public <T> EventBuilder setPayload(AnyValue<T> payload) {
this.payload.clear();
((ExtendedLogRecordBuilder) logRecordBuilder).setBody(payload);
Comment on lines +40 to +41
Copy link
Author

@breedx-splk breedx-splk Mar 26, 2024

Choose a reason for hiding this comment

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

This might be the grossest part of this. It's also a little weird of an API because the builder could get reused and it's a bit of a chimera -- handling both simple types and complex types. Like, a confused user might call put(), put(), put() then setPayload() then put(), put(), put() again.

Not sure why you'd ever want to or why one might think that should work, but it's possible. Just wanted to point it out.

Copy link
Owner

@jack-berg jack-berg Mar 26, 2024

Choose a reason for hiding this comment

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

handling both simple types and complex types. Like, a confused user might call put(), put(), put() then setPayload() then put(), put(), put() again.

Yeah in my head we'd solve this by having a nullable reference to the user set AnyValue payload. And then down in emit, if explicit payload was set, it takes priority over any calls to put. Something like:

@Nullable private AnyValue<?> explicitPayload;

...

public EventBuilder setPayload(AnyValue<?> payload) {
  this.explicitPayload = payloadl;
  return this;
}

...

@Override
public void emit() {
  if (explicitPayload != null) {
     ((ExtendedLogRecordBuilder) logRecordBuilder).setBody(explicitPayload);
  } else if (!payload.isEmpty()) {
    ((ExtendedLogRecordBuilder) logRecordBuilder).setBody(AnyValue.of(payload));
  }
  if (!hasTimestamp) {
    logRecordBuilder.setTimestamp(clock.now(), TimeUnit.NANOSECONDS);
  }
  logRecordBuilder.setAttribute(EVENT_NAME, eventName);
  logRecordBuilder.emit();
}

Potential to cause confusion to users, but can mitigate with javadoc explaining the behavior. You might say that this might be cause to remove / change the put(String, ?) methods, but I think that those are absolutely crucial from an ergonomics perspective.

Copy link
Author

@breedx-splk breedx-splk Mar 26, 2024

Choose a reason for hiding this comment

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

Yeah that's cool, the null was another approach that occurred to me, and I think the usability is fine and maybe slightly cleaner impl. I agree that those put() calls cannot go away!

return this;
}

@Override
public EventBuilder put(String key, AnyValue<?> value) {
payload.put(key, value);
Expand Down
10 changes: 10 additions & 0 deletions sdk/logs/src/main/java/io/opentelemetry/sdk/logs/internal/SdkEventLoggerProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@

package io.opentelemetry.sdk.logs.internal;

import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.api.incubator.events.EventBuilder;
import io.opentelemetry.api.incubator.events.EventLogger;
import io.opentelemetry.api.incubator.events.EventLoggerBuilder;
import io.opentelemetry.api.incubator.events.EventLoggerProvider;
import io.opentelemetry.api.incubator.logs.AnyValue;
import io.opentelemetry.api.logs.Logger;
import io.opentelemetry.api.logs.LoggerBuilder;
import io.opentelemetry.api.logs.LoggerProvider;
Expand Down Expand Up @@ -97,6 +99,14 @@ private SdkEventLogger(Clock clock, Logger delegateLogger) {
this.delegateLogger = delegateLogger;
}

@Override
public <T> void emit(String eventName, AnyValue<T> payload, Attributes attributes) {
builder(eventName)
.setAttributes(attributes)
.setPayload(payload)
.emit();
}

@Override
public EventBuilder builder(String eventName) {
return new SdkEventBuilder(
Expand Down