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

Remove domain from event api. #6253

Merged
merged 5 commits into from
Mar 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,6 @@ public EventEmitterBuilder setInstrumentationVersion(String instrumentationVersi
return this;
}

@Override
public EventEmitterBuilder setEventDomain(String eventDomain) {
return this;
}

@Override
public EventEmitter build() {
return DefaultEventEmitter.getInstance();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
*
* <pre>{@code
* class MyClass {
* private final EventEmitter eventEmitter = openTelemetryEventEmitterProvider.eventEmitterBuilder("scope-name")
* .setEventDomain("acme.observability")
* private final EventEmitter eventEmitter = openTelemetryEventEmitterProvider
* .eventEmitterBuilder("scope-name")
* .build();
*
* void doWork() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,6 @@
*/
public interface EventEmitterBuilder {

/**
* Sets the event domain. Event domain is not part of {@link EventEmitter} identity.
*
* @param eventDomain The event domain, which acts as a namespace for event names. Within a
* particular event domain, event name defines a particular class or type of event.
* @return this
*/
EventEmitterBuilder setEventDomain(String eventDomain);

/**
* Set the scope schema URL of the resulting {@link EventEmitter}. Schema URL is part of {@link
* EventEmitter} identity.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ void noopEventEmitterProvider_doesNotThrow() {
() ->
provider
.eventEmitterBuilder("scope-name")
.setEventDomain("event-domain")
.setInstrumentationVersion("1.0")
.setSchemaUrl("http://schema.com")
.build())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ void emit() {
assertThatCode(
() ->
DefaultEventEmitter.getInstance()
.emit("event-name", Attributes.builder().put("key1", "value1").build()))
.emit(
"event-domain.event-name",
Attributes.builder().put("key1", "value1").build()))
.doesNotThrowAnyException();
}

Expand All @@ -32,7 +34,7 @@ void builder() {
assertThatCode(
() ->
emitter
.builder("myEvent", attributes)
.builder("com.example.MyEvent", attributes)
.setTimestamp(123456L, TimeUnit.NANOSECONDS)
.setTimestamp(Instant.now())
.emit())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -536,7 +536,6 @@ private static void testLogRecordExporter(LogRecordExporter logRecordExporter) {
EventEmitter eventEmitter =
SdkEventEmitterProvider.create(loggerProvider)
.eventEmitterBuilder(OtlpExporterIntegrationTest.class.getName())
.setEventDomain("event-domain")
.build();

SpanContext spanContext =
Expand Down Expand Up @@ -712,10 +711,6 @@ private static void testLogRecordExporter(LogRecordExporter logRecordExporter) {
assertThat(protoLog2.getBody().getStringValue()).isEmpty();
assertThat(protoLog2.getAttributesList())
.containsExactlyInAnyOrder(
KeyValue.newBuilder()
.setKey("event.domain")
.setValue(AnyValue.newBuilder().setStringValue("event-domain").build())
.build(),
KeyValue.newBuilder()
.setKey("event.name")
.setValue(AnyValue.newBuilder().setStringValue("event-name").build())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,10 +207,7 @@ void configures() throws Exception {
logger.logRecordBuilder().setBody("info log message").setSeverity(Severity.INFO).emit();

EventEmitter eventEmitter =
GlobalEventEmitterProvider.get()
.eventEmitterBuilder("test")
.setEventDomain("test-domain")
.build();
GlobalEventEmitterProvider.get().eventEmitterBuilder("test").build();
eventEmitter.emit("test-name", Attributes.builder().put("cow", "moo").build());

openTelemetrySdk.getSdkTracerProvider().forceFlush().join(10, TimeUnit.SECONDS);
Expand Down Expand Up @@ -336,10 +333,6 @@ void configures() throws Exception {
logRecord ->
assertThat(logRecord.getAttributesList())
.containsExactlyInAnyOrder(
KeyValue.newBuilder()
.setKey("event.domain")
.setValue(AnyValue.newBuilder().setStringValue("test-domain").build())
.build(),
KeyValue.newBuilder()
.setKey("event.name")
.setValue(AnyValue.newBuilder().setStringValue("test-name").build())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,10 @@

class SdkEventBuilder implements EventBuilder {
private final LogRecordBuilder logRecordBuilder;
private final String eventDomain;
private final String eventName;

SdkEventBuilder(LogRecordBuilder logRecordBuilder, String eventDomain, String eventName) {
SdkEventBuilder(LogRecordBuilder logRecordBuilder, String eventName) {
this.logRecordBuilder = logRecordBuilder;
this.eventDomain = eventDomain;
this.eventName = eventName;
}

Expand All @@ -35,7 +33,7 @@ public EventBuilder setTimestamp(Instant instant) {

@Override
public void emit() {
SdkEventEmitterProvider.addEventNameAndDomain(logRecordBuilder, eventDomain, eventName);
SdkEventEmitterProvider.addEventName(logRecordBuilder, eventName);
logRecordBuilder.emit();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,8 @@
*/
public final class SdkEventEmitterProvider implements EventEmitterProvider {

static final AttributeKey<String> EVENT_DOMAIN = AttributeKey.stringKey("event.domain");
static final AttributeKey<String> EVENT_NAME = AttributeKey.stringKey("event.name");

static final String DEFAULT_EVENT_DOMAIN = "unknown";

private final LoggerProvider delegateLoggerProvider;
private final Clock clock;

Expand All @@ -55,9 +52,7 @@

@Override
public EventEmitter get(String instrumentationScopeName) {
return eventEmitterBuilder(instrumentationScopeName)
.setEventDomain(DEFAULT_EVENT_DOMAIN)
.build();
return eventEmitterBuilder(instrumentationScopeName).build();

Check warning on line 55 in sdk/logs/src/main/java/io/opentelemetry/sdk/logs/internal/SdkEventEmitterProvider.java

View check run for this annotation

Codecov / codecov/patch

sdk/logs/src/main/java/io/opentelemetry/sdk/logs/internal/SdkEventEmitterProvider.java#L55

Added line #L55 was not covered by tests
}

@Override
Expand All @@ -70,19 +65,12 @@

private final Clock clock;
private final LoggerBuilder delegateLoggerBuilder;
private String eventDomain = DEFAULT_EVENT_DOMAIN;

private SdkEventEmitterBuilder(Clock clock, LoggerBuilder delegateLoggerBuilder) {
this.clock = clock;
this.delegateLoggerBuilder = delegateLoggerBuilder;
}

@Override
public EventEmitterBuilder setEventDomain(String eventDomain) {
this.eventDomain = eventDomain;
return this;
}

@Override
public EventEmitterBuilder setSchemaUrl(String schemaUrl) {
delegateLoggerBuilder.setSchemaUrl(schemaUrl);
Expand All @@ -97,20 +85,18 @@

@Override
public EventEmitter build() {
return new SdkEventEmitter(clock, delegateLoggerBuilder.build(), eventDomain);
return new SdkEventEmitter(clock, delegateLoggerBuilder.build());
}
}

private static class SdkEventEmitter implements EventEmitter {

private final Clock clock;
private final Logger delegateLogger;
private final String eventDomain;

private SdkEventEmitter(Clock clock, Logger delegateLogger, String eventDomain) {
private SdkEventEmitter(Clock clock, Logger delegateLogger) {
this.clock = clock;
this.delegateLogger = delegateLogger;
this.eventDomain = eventDomain;
}

@Override
Expand All @@ -120,7 +106,6 @@
.logRecordBuilder()
.setTimestamp(clock.now(), TimeUnit.NANOSECONDS)
.setAllAttributes(attributes),
eventDomain,
eventName);
}

Expand All @@ -131,13 +116,12 @@
.logRecordBuilder()
.setTimestamp(clock.now(), TimeUnit.NANOSECONDS)
.setAllAttributes(attributes);
addEventNameAndDomain(logRecordBuilder, eventDomain, eventName);
addEventName(logRecordBuilder, eventName);
logRecordBuilder.emit();
}
}

static void addEventNameAndDomain(
LogRecordBuilder logRecordBuilder, String eventDomain, String eventName) {
logRecordBuilder.setAttribute(EVENT_DOMAIN, eventDomain).setAttribute(EVENT_NAME, eventName);
static void addEventName(LogRecordBuilder logRecordBuilder, String eventName) {
logRecordBuilder.setAttribute(EVENT_NAME, eventName);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@
import static io.opentelemetry.api.common.AttributeKey.stringKey;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

Expand All @@ -21,19 +24,18 @@ class SdkEventBuilderTest {

@Test
void emit() {
String eventDomain = "mydomain";
String eventName = "banana";

LogRecordBuilder logRecordBuilder = mock(LogRecordBuilder.class);
when(logRecordBuilder.setTimestamp(anyLong(), any())).thenReturn(logRecordBuilder);
when(logRecordBuilder.setAttribute(any(), any())).thenReturn(logRecordBuilder);

Instant instant = Instant.now();
new SdkEventBuilder(logRecordBuilder, eventDomain, eventName)
new SdkEventBuilder(logRecordBuilder, eventName)
.setTimestamp(123456L, TimeUnit.NANOSECONDS)
.setTimestamp(instant)
.emit();
verify(logRecordBuilder).setAttribute(stringKey("event.domain"), eventDomain);
verify(logRecordBuilder, never()).setAttribute(eq(stringKey("event.domain")), anyString());
verify(logRecordBuilder).setAttribute(stringKey("event.name"), eventName);
verify(logRecordBuilder).setTimestamp(123456L, TimeUnit.NANOSECONDS);
verify(logRecordBuilder).setTimestamp(instant);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,62 +37,26 @@ class SdkEventEmitterProviderTest {
clock);

@Test
void emit_WithDomain() {
void emit() {
when(clock.now()).thenReturn(10L);

eventEmitterProvider
.eventEmitterBuilder("test-scope")
.setEventDomain("event-domain")
.build()
.emit(
"event-name",
Attributes.builder()
.put("key1", "value1")
// should be overridden by the eventName argument passed to emit
.put("event.name", "foo")
// should be overridden by the eventDomain
.put("event.domain", "foo")
.build());

assertThat(seenLog.get().toLogRecordData())
.hasResource(RESOURCE)
.hasInstrumentationScope(InstrumentationScopeInfo.create("test-scope"))
.hasTimestamp(10L)
.hasAttributes(
Attributes.builder()
.put("key1", "value1")
.put("event.domain", "event-domain")
.put("event.name", "event-name")
.build());
}

@Test
void emit_NoDomain() {
when(clock.now()).thenReturn(10L);

eventEmitterProvider
.eventEmitterBuilder("test-scope")
.build()
.emit(
"event-name",
Attributes.builder()
.put("key1", "value1")
// should be overridden by the eventName argument passed to emit
.put("event.name", "foo")
// should be overridden by the default eventDomain
.put("event.domain", "foo")
.build());

assertThat(seenLog.get().toLogRecordData())
.hasResource(RESOURCE)
.hasInstrumentationScope(InstrumentationScopeInfo.create("test-scope"))
.hasTimestamp(10L)
.hasAttributes(
Attributes.builder()
.put("key1", "value1")
.put("event.domain", "unknown")
.put("event.name", "event-name")
.build());
Attributes.builder().put("key1", "value1").put("event.name", "event-name").build());
}

@Test
Expand All @@ -111,10 +75,6 @@ private void verifySeen(long timestamp, Attributes attributes) {
.hasResource(RESOURCE)
.hasInstrumentationScope(InstrumentationScopeInfo.create("test-scope"))
.hasTimestamp(timestamp)
.hasAttributes(
attributes.toBuilder()
.put("event.domain", "unknown")
.put("event.name", "testing")
.build());
.hasAttributes(attributes.toBuilder().put("event.name", "testing").build());
}
}