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.
Comment From: markpollack
@jonatan-ivanov can you review this please?
Comment From: jonatan-ivanov
gen_ai.operation.name
is there as an enum:
https://github.com/spring-projects/spring-ai/blob/efc392394ccc890b7b5b93e7207c52e3d6cc6dd0/spring-ai-core/src/main/java/org/springframework/ai/observation/conventions/AiObservationAttributes.java#L38
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: https://github.com/spring-projects/spring-ai/blob/efc392394ccc890b7b5b93e7207c52e3d6cc6dd0/spring-ai-core/src/main/java/org/springframework/ai/chat/client/observation/DefaultChatClientObservationConvention.java#L63-L71
It seems spring.ai.advisor
is not a tag on the metric but the name of it:
https://github.com/spring-projects/spring-ai/blob/efc392394ccc890b7b5b93e7207c52e3d6cc6dd0/spring-ai-core/src/main/java/org/springframework/ai/chat/client/advisor/observation/DefaultAdvisorObservationConvention.java#L34-L40
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?
Comment From: habuma
It's been awhile since I created this and thought about it, but..
It's not that I don't see the spring.ai.advisor
metric. I do see it when stuff flows through an advisor. It's just that I never see those two keys. For example, I just ran a request through ChatClient
in a situation where there are a few advisors in play and here's what I got back from spring.ai.advisor
:
"name": "spring.ai.advisor",
"baseUnit": "seconds",
"measurements": [
{
"statistic": "COUNT",
"value": 3.0
},
{
"statistic": "TOTAL_TIME",
"value": 7.119742541999999
},
{
"statistic": "MAX",
"value": 4.183466542
}
],
"availableTags": [
{
"tag": "spring.ai.advisor.type",
"values": [
"AROUND"
]
},
{
"tag": "spring.ai.kind",
"values": [
"advisor"
]
},
{
"tag": "spring.ai.advisor.name",
"values": [
"CallAroundAdvisor",
"VectorStoreChatMemoryAdvisor",
"QuestionAnswerAdvisor"
]
},
{
"tag": "error",
"values": [
"none"
]
}
]
}
Neither of the two keys I mentioned are in this response. Since they are documented as low-cardinality keys, I'd expect them to be there. And yes, I see from the code you showed where aiOperationType()
should've put the gen_ai.operation.name
key in there, so I'm confused why I'm not seeing it. And I see nothing to suggest that the gen_ai.system
should've been included.
Also, this most recent attempt was done moments ago using a local build of the very latest snapshot code (to take advantage of the fix for #1738 / #1742 ).
Comment From: ThomasVitale
There's probably some error in the code if those keys are not populated, good catch! The reason for having those low cardinality keys is to enable the integration of Spring AI with observability/evaluation platforms following the OpenTelemetry Semantic Conventions for Generative AI Systems. I'll work soon on a sample app demonstrating integrations with some of those platforms with the hope to clarify the usage and review the current observability setup in Spring AI.
The gen_ai.operation.name
is named like that because of the Semantic Conventions, but the name is poorly chosen, so that's why internally in Spring AI we call that an "operation type". We did something similar for gen_ai.system
, which we call a "provider" in Spring AI when it's meant to represent a model provider ("system" is not a good name, too generic).
Comment From: ilayaperumalg
@habuma Closed this PR as the underlying issue is fixed now - both the keys are updated for the Advisor observation.