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

Events in DocumentedObservation #3367

Merged
merged 4 commits into from
Aug 25, 2022
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ void handlerSupportsAnyContext() {
assertThatCode(() -> handler.onStart(testContext)).doesNotThrowAnyException();
assertThatCode(() -> handler.onStop(testContext)).doesNotThrowAnyException();
assertThatCode(() -> handler.onError(testContext)).doesNotThrowAnyException();
assertThatCode(() -> handler.onEvent(new Observation.Event("testEvent"), testContext))
assertThatCode(() -> handler.onEvent(Observation.Event.of("testEvent"), testContext))
.doesNotThrowAnyException();
assertThatCode(() -> handler.onScopeOpened(testContext)).doesNotThrowAnyException();
assertThatCode(() -> handler.supportsContext(testContext)).doesNotThrowAnyException();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ void handlerSupportsConcreteContextForHandlerMethods() {
assertThatCode(() -> handler.onStart(context())).doesNotThrowAnyException();
assertThatCode(() -> handler.onStop(context())).doesNotThrowAnyException();
assertThatCode(() -> handler.onError(context())).doesNotThrowAnyException();
assertThatCode(() -> handler.onEvent(new Observation.Event("testEvent"), context())).doesNotThrowAnyException();
assertThatCode(() -> handler.onEvent(Observation.Event.of("testEvent"), context())).doesNotThrowAnyException();
assertThatCode(() -> handler.onScopeOpened(context())).doesNotThrowAnyException();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ void observeWithHandlers() {
inOrder.verify(handler).onScopeOpened(isA(Observation.Context.class));
assertThat(scope.getCurrentObservation()).isSameAs(observation);

Observation.Event event = new Observation.Event("testEvent", "event for testing");
Observation.Event event = Observation.Event.of("testEvent", "event for testing");
observation.event(event);
inOrder.verify(handler).onEvent(same(event), isA(Observation.Context.class));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -916,58 +916,6 @@ private String toString(Map<Object, Object> map) {

}

/**
* An arbitrary event that you can extend and signal during an {@link Observation}.
* This helps you to tell to the {@link ObservationHandler} that something happened.
* If you want to signal an exception/error, please use
* {@link Observation#error(Throwable)} instead.
*/
class Event {

private final String name;

private final String contextualName;

/**
* @param name The name of the event.
*/
public Event(String name) {
this(name, name);
}

/**
* @param name The name of the event (should have low cardinality).
* @param contextualName The contextual name of the event (can have high
* cardinality).
*/
public Event(String name, String contextualName) {
this.name = name;
this.contextualName = contextualName;
}

/**
* Returns the name of the event.
* @return the name of the event.
*/
public String getName() {
return this.name;
}

/**
* Returns the contextual name of the event.
* @return the contextual name of the event.
*/
public String getContextualName() {
return this.contextualName;
}

@Override
public String toString() {
return "event.name='" + this.name + "', event.contextualName='" + this.contextualName + '\'';
}

}

/**
* Read only view on the {@link Context}.
*/
Expand Down Expand Up @@ -1158,4 +1106,81 @@ interface CheckedCallable<T, E extends Throwable> {

}

/**
* Represents an event used for documenting instrumentation.
*
* @author Marcin Grzejszczak
* @since 1.10.0
*/
interface Event {

/**
* Returns event name.
* @return event name
*/
String getName();

/**
* Returns event contextual name. You can use {@code %s} to represent dynamic
* entries that should be resolved at runtime via
* {@link String#format(String, Object...)}.
* @return event contextual name
*/
default String getContextualName() {
return getName();
}

/**
* Creates a {@link Event} for the given names.
* @param name event name
* @param contextualName event contextual name
* @return event
*/
static Event of(String name, String contextualName) {
return new Event() {
@Override
public String getName() {
return name;
}

@Override
public String getContextualName() {
return contextualName;
}
};
}

/**
* Creates a {@link Event} for the given name.
* @param name event name
* @return event
*/
static Event of(String name) {
return of(name, name);
}

/**
* Creates an event for the given key name.
* @param dynamicEntriesForContextualName variables to be resolved in
* {@link Event#getContextualName()} via {@link String#format(String, Object...)}
* @return event
*/
default Event format(Object... dynamicEntriesForContextualName) {
Event parent = this;
return new Event() {

@Override
public String getName() {
return parent.getName();
}

@Override
public String getContextualName() {
return String.format(parent.getContextualName(), dynamicEntriesForContextualName);
}
};
}

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class SimpleObservation implements Observation {

private final Collection<ObservationFilter> filters;

SimpleObservation(String name, ObservationRegistry registry, Context context) {
SimpleObservation(@Nullable String name, ObservationRegistry registry, Context context) {
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure that this can be null here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I mean look at line 60, we literally null the name

Copy link
Member

Choose a reason for hiding this comment

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

I get that a DocumentedObservation's getName() can return null for filtering purposes but if I take a step back, that might be valid for the DocumentedObservation but I think SimpleObservation should not accept null here. What should the meter handler or the tracing handler do in that case?

Also, DocumentedObservation has a null-check so that nullability behavior can be isolated there I guess.

this.registry = registry;
this.context = context;
this.context.setName(name);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,17 @@ public interface DocumentedObservation {
*/
KeyName[] EMPTY = new KeyName[0];

/**
* Empty event names.
*/
Observation.Event[] EMPTY_EVENT_NAMES = new Observation.Event[0];

/**
* Default technical name (e.g.: metric name). You can set the name either by this
* method or {@link #getDefaultConvention()}. You can't use both.
* @return name
*/
@Nullable
default String getName() {
return null;
}
Expand Down Expand Up @@ -101,6 +107,14 @@ default KeyName[] getHighCardinalityKeyNames() {
return EMPTY;
}

/**
* Event values.
* @return allowed event values
*/
default Observation.Event[] getEvents() {
return EMPTY_EVENT_NAMES;
}

/**
* Returns required prefix to be there for tags. For example, {@code foo.} would
* require the tags to have a {@code foo.} prefix like this: {@code foo.bar=true}.
Expand Down Expand Up @@ -163,6 +177,9 @@ default <T extends Observation.Context> Observation observation(
+ "] but you have provided an incompatible one of type [" + defaultConvention.getClass() + "]");
}
Observation observation = Observation.createNotStarted(customConvention, defaultConvention, context, registry);
if (getName() != null) {
context.setName(getName());
}
if (getContextualName() != null) {
observation.contextualName(getContextualName());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ void should_run_on_event_for_all_matching_handlers() {
AllMatchingCompositeObservationHandler allMatchingHandler = new AllMatchingCompositeObservationHandler(
new NotMatchingHandler(), this.matchingHandler, new NotMatchingHandler(), this.matchingHandler2);

allMatchingHandler.onEvent(new Observation.Event("testEvent"), null);
allMatchingHandler.onEvent(Observation.Event.of("testEvent"), null);

assertThat(this.matchingHandler.eventDetected).isTrue();
assertThat(this.matchingHandler2.eventDetected).isTrue();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ void should_run_on_event_only_for_first_matching_handler() {
FirstMatchingCompositeObservationHandler firstMatchingHandler = new FirstMatchingCompositeObservationHandler(
new NotMatchingHandler(), this.matchingHandler, new NotMatchingHandler());

firstMatchingHandler.onEvent(new Observation.Event("testEvent"), null);
firstMatchingHandler.onEvent(Observation.Event.of("testEvent"), null);

assertThat(this.matchingHandler.eventDetected).isTrue();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -642,7 +642,7 @@ void recordWithObservation() {
observation.lowCardinalityKeyValue("dynamicTag", "24");

clock(registry).add(1, TimeUnit.SECONDS);
observation.event(new Observation.Event("testEvent", "event for testing"));
observation.event(Observation.Event.of("testEvent", "event for testing"));

LongTaskTimer longTaskTimer = registry.more().longTaskTimer("myObservation.active", "staticTag", "42");
assertThat(longTaskTimer.activeTasks()).isEqualTo(1);
Expand All @@ -667,7 +667,7 @@ void recordWithObservationAndScope() {
try (Observation.Scope scope = observation.openScope()) {
assertThat(scope.getCurrentObservation()).isSameAs(observation);
clock(registry).add(10, TimeUnit.NANOSECONDS);
observation.event(new Observation.Event("testEvent", "event for testing"));
observation.event(Observation.Event.of("testEvent", "event for testing"));
}
observation.stop();
clock(registry).add(step());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public static void main(String[] args) throws InterruptedException {

try (Observation.Scope scope = observation.openScope()) {
Thread.sleep(1_000);
observation.event(new Observation.Event("custom.event", "Custom " + UUID.randomUUID()));
observation.event(Observation.Event.of("custom.event", "Custom " + UUID.randomUUID()));
observation.error(new IOException("simulated"));
}

Expand Down