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 advisor low-cardinality keys that aren't actually exposed. #1660

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

habuma
Copy link
Member

@habuma habuma commented Nov 3, 2024

I noticed that gen_ai.operation.name and gen_ai.system are shown as low-cardinality keys for spring.ai.advisor, but neither of those appear in the metrics. Moreover, I confirmed that are aren't included as low-cardinality keys in either DefaultAdvisorObservationConvention or AdvisorObservationDocumentation. So this PR removes them from the documentation.

@markpollack
Copy link
Member

@jonatan-ivanov can you review this please?

@markpollack markpollack added the documentation Improvements or additions to documentation label Nov 4, 2024
@jonatan-ivanov
Copy link
Member

gen_ai.operation.name is there as an enum:

But it is called AI_OPERATION_TYPE. Type vs. name puzzles me a bit (it seems it is called type everywhere except that value in the enum). It also seems to be used in certain cases, there are even integration tests for it. I would follow through the usage since it seems DefaultChatClientObservationConvention uses it (with other conventions) and if the keyvalue is added metrics should also have this tag:

@Override
public KeyValues getLowCardinalityKeyValues(ChatClientObservationContext context) {
return KeyValues.of(aiOperationType(context), aiProvider(context), springAiKind(), stream(context));
}
protected KeyValue aiOperationType(ChatClientObservationContext context) {
return KeyValue.of(ChatModelObservationDocumentation.LowCardinalityKeyNames.AI_OPERATION_TYPE,
context.getOperationMetadata().operationType());
}

It seems spring.ai.advisor is not a tag on the metric but the name of it:

public static final String DEFAULT_NAME = "spring.ai.advisor";
private final String name;
public DefaultAdvisorObservationConvention() {
this(DEFAULT_NAME);
}

Though metrics are only created when you trigger the operation that records the observation, can it happen that your use-case did not trigger it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants