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

Add getContextualName(Context c) on Observation.Convention #3301

Merged
merged 2 commits into from
Jul 20, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
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 @@ -152,12 +152,13 @@ static Observation start(ObservationConvention<?> observationConvention, @Nullab
/**
* Creates and starts an {@link Observation}. When no registry is passed or
* observation is not applicable will return a no-op observation.
* @param <T> type of context
* @param observationConvention observation convention
* @param context mutable context
* @param registry observation registry
* @return started observation
*/
static Observation start(ObservationConvention<?> observationConvention, @Nullable Context context,
static <T extends Context> Observation start(ObservationConvention<T> observationConvention, @Nullable T context,
@Nullable ObservationRegistry registry) {
return createNotStarted(observationConvention, context, registry).start();
}
Expand Down Expand Up @@ -204,19 +205,25 @@ static Observation createNotStarted(ObservationConvention<?> observationConventi
* {@link Observation#start()} when you want the measurements to start. When no
* registry is passed or observation is not applicable will return a no-op
* observation.
* @param <T> type of context
* @param observationConvention observation convention
* @param context mutable context
* @param registry observation registry
* @return created but not started observation
*/
static Observation createNotStarted(ObservationConvention<?> observationConvention, @Nullable Context context,
@Nullable ObservationRegistry registry) {
static <T extends Observation.Context> Observation createNotStarted(ObservationConvention<T> observationConvention,
@Nullable T context, @Nullable ObservationRegistry registry) {
if (registry == null || registry.isNoop()
|| !registry.observationConfig().isObservationEnabled(observationConvention.getName(), context)
|| observationConvention == NoopObservationConvention.INSTANCE) {
return NoopObservation.INSTANCE;
}
return new SimpleObservation(observationConvention, registry, context == null ? new Context() : context);
Context c = context == null ? new Context() : context;
SimpleObservation simpleObservation = new SimpleObservation(observationConvention, registry, c);
if (context != null) {
simpleObservation.contextualName(observationConvention.getContextualName(context));
}
return simpleObservation;
}

/**
Expand Down Expand Up @@ -939,6 +946,17 @@ interface ObservationConvention<T extends Observation.Context>
*/
String getName();

/**
* Allows to override the contextual name for an observation. The Observation will
* be renamed only when an explicit context was passed - if an implicit context is
Copy link
Member

@shakuzen shakuzen Jul 20, 2022

Choose a reason for hiding this comment

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

I think the documentation about when the contextual name will be overridden might be better on Observation#createNotStarted since that is where the mentioned logic is. A user could use this method in other ways.

* used this method won't be called.
* @param context context
* @return the new, contextual name for the observation
*/
default String getContextualName(T context) {
return null;
}

}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,16 +65,19 @@ default String getName() {
}

/**
* Default naming convention (sets a technical name and key values). You can set the
* name either by this method or {@link #getName()} ()}. You can't use both.
* Default naming convention (sets a technical and contextual names, and key values).
* You can set the names either by this method or {@link #getName()} and
* {@link #getContextualName()}.
* @return default naming convention
*/
default Class<? extends Observation.ObservationConvention<? extends Observation.Context>> getDefaultConvention() {
return null;
}

/**
* More human readable name available within the given context (e.g. span name).
* More human readable name available within the given context (e.g. span name). You
* can set the name either by this method or {@link #getDefaultConvention()}. This
* method will override what {@link #getDefaultConvention()} has set.
* @return contextual name
*/
default String getContextualName() {
Expand Down Expand Up @@ -154,8 +157,11 @@ else if (!getDefaultConvention()
+ "] defined default convention to be of type [" + getDefaultConvention()
+ "] but you have provided an incompatible one of type [" + defaultConvention.getClass() + "]");
}
return Observation.createNotStarted(customConvention, defaultConvention, context, registry)
.contextualName(getContextualName());
Observation observation = Observation.createNotStarted(customConvention, defaultConvention, context, registry);
if (getContextualName() != null) {
observation.contextualName(getContextualName());
}
return observation;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,18 @@ void observationShouldBeCreatedWhenObservationConventionIsOfProperType() {
then(context.getHighCardinalityKeyValues()).isEqualTo(KeyValues.of("high key", "high value"));
}

@Test
void contextualNameShouldBeOverridden() {
ObservationRegistry registry = observationRegistry();
Observation.Context context = new Observation.Context();

TestConventionObservation.CONTEXTUAL_NAME.observation(null, new ContextualObservation(), context, registry)
.start().stop();

then(context.getName()).isEqualTo("technical name");
then(context.getContextualName()).isEqualTo("contextual name");
}

private ObservationRegistry observationRegistry() {
ObservationRegistry registry = ObservationRegistry.create();
registry.observationConfig().observationHandler(context -> true);
Expand All @@ -85,6 +97,14 @@ public String getContextualName() {
return "contextual";
}

@Override
public Class<? extends Observation.ObservationConvention<? extends Observation.Context>> getDefaultConvention() {
return FirstObservationConvention.class;
}
},

CONTEXTUAL_NAME {

@Override
public Class<? extends Observation.ObservationConvention<? extends Observation.Context>> getDefaultConvention() {
return FirstObservationConvention.class;
Expand Down Expand Up @@ -145,4 +165,33 @@ public boolean supportsContext(Observation.Context context) {

}

static class ContextualObservation extends FirstObservationConvention {

@Override
public String getName() {
return "technical name";
}

@Override
public String getContextualName(Observation.Context context) {
return "contextual name";
}

@Override
public KeyValues getLowCardinalityKeyValues(Observation.Context context) {
return KeyValues.of("low key", "low value");
}

@Override
public KeyValues getHighCardinalityKeyValues(Observation.Context context) {
return KeyValues.of("high key", "high value");
}

@Override
public boolean supportsContext(Observation.Context context) {
return true;
}

}

}