From d096e1899937dce20cfb1afa1c215dfdfa669230 Mon Sep 17 00:00:00 2001 From: Christian Tzolov Date: Tue, 12 Nov 2024 15:32:44 +0100 Subject: [PATCH] feat: Move the advisor name to low cardinality key values - Adviser name is moved from hight to low cardinality key values. - Updated tests in to reflect these changes, ensuring the new low cardinality key is correctly captured. Resolves #1716 --- .../AdvisorObservationDocumentation.java | 11 ++++++----- .../DefaultAdvisorObservationConvention.java | 15 +++++++-------- .../DefaultAdvisorObservationConventionTests.java | 2 +- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/spring-ai-core/src/main/java/org/springframework/ai/chat/client/advisor/observation/AdvisorObservationDocumentation.java b/spring-ai-core/src/main/java/org/springframework/ai/chat/client/advisor/observation/AdvisorObservationDocumentation.java index 835dfe614fc..3d1080fe8b2 100644 --- a/spring-ai-core/src/main/java/org/springframework/ai/chat/client/advisor/observation/AdvisorObservationDocumentation.java +++ b/spring-ai-core/src/main/java/org/springframework/ai/chat/client/advisor/observation/AdvisorObservationDocumentation.java @@ -68,11 +68,7 @@ public String asString() { public String asString() { return "spring.ai.advisor.type"; } - } - - } - - public enum HighCardinalityKeyNames implements KeyName { + }, /** * Advisor name. @@ -83,6 +79,11 @@ public String asString() { return "spring.ai.advisor.name"; } }, + + } + + public enum HighCardinalityKeyNames implements KeyName { + /** * Advisor order in the advisor chain. */ diff --git a/spring-ai-core/src/main/java/org/springframework/ai/chat/client/advisor/observation/DefaultAdvisorObservationConvention.java b/spring-ai-core/src/main/java/org/springframework/ai/chat/client/advisor/observation/DefaultAdvisorObservationConvention.java index 2b4ce63cd4e..e28fb7057e8 100644 --- a/spring-ai-core/src/main/java/org/springframework/ai/chat/client/advisor/observation/DefaultAdvisorObservationConvention.java +++ b/spring-ai-core/src/main/java/org/springframework/ai/chat/client/advisor/observation/DefaultAdvisorObservationConvention.java @@ -62,7 +62,7 @@ public String getContextualName(AdvisorObservationContext context) { @Override public KeyValues getLowCardinalityKeyValues(AdvisorObservationContext context) { - return KeyValues.of(springAiKind(), advisorType(context)); + return KeyValues.of(springAiKind(), advisorType(context), advisorName(context)); } protected KeyValue advisorType(AdvisorObservationContext context) { @@ -70,8 +70,11 @@ protected KeyValue advisorType(AdvisorObservationContext context) { } protected KeyValue springAiKind() { - return KeyValue.of(AdvisorObservationDocumentation.LowCardinalityKeyNames.SPRING_AI_KIND, - SpringAiKind.ADVISOR.value()); + return KeyValue.of(LowCardinalityKeyNames.SPRING_AI_KIND, SpringAiKind.ADVISOR.value()); + } + + protected KeyValue advisorName(AdvisorObservationContext context) { + return KeyValue.of(LowCardinalityKeyNames.ADVISOR_NAME, context.getAdvisorName()); } // ------------------------ @@ -80,11 +83,7 @@ protected KeyValue springAiKind() { @Override public KeyValues getHighCardinalityKeyValues(AdvisorObservationContext context) { - return KeyValues.of(advisorName(context), advisorOrder(context)); - } - - protected KeyValue advisorName(AdvisorObservationContext context) { - return KeyValue.of(HighCardinalityKeyNames.ADVISOR_NAME, context.getAdvisorName()); + return KeyValues.of(advisorOrder(context)); } protected KeyValue advisorOrder(AdvisorObservationContext context) { diff --git a/spring-ai-core/src/test/java/org/springframework/ai/chat/client/advisor/observation/DefaultAdvisorObservationConventionTests.java b/spring-ai-core/src/test/java/org/springframework/ai/chat/client/advisor/observation/DefaultAdvisorObservationConventionTests.java index 906a3376873..d1fe2c291bc 100644 --- a/spring-ai-core/src/test/java/org/springframework/ai/chat/client/advisor/observation/DefaultAdvisorObservationConventionTests.java +++ b/spring-ai-core/src/test/java/org/springframework/ai/chat/client/advisor/observation/DefaultAdvisorObservationConventionTests.java @@ -69,6 +69,7 @@ void shouldHaveLowCardinalityKeyValuesWhenDefined() { assertThat(this.observationConvention.getLowCardinalityKeyValues(observationContext)).contains( KeyValue.of(LowCardinalityKeyNames.ADVISOR_TYPE.asString(), AdvisorObservationContext.Type.AROUND.name()), + KeyValue.of(LowCardinalityKeyNames.ADVISOR_NAME.asString(), "MyName"), KeyValue.of(LowCardinalityKeyNames.SPRING_AI_KIND.asString(), SpringAiKind.ADVISOR.value())); } @@ -81,7 +82,6 @@ void shouldHaveKeyValuesWhenDefinedAndResponse() { .build(); assertThat(this.observationConvention.getHighCardinalityKeyValues(observationContext)) - .contains(KeyValue.of(HighCardinalityKeyNames.ADVISOR_NAME.asString(), "MyName")) .contains(KeyValue.of(HighCardinalityKeyNames.ADVISOR_ORDER.asString(), "678")); }