Skip to content

aws_bedrock: retain contextualGroundingPolicy check details#11890

Merged
efd6 merged 0 commit intoelastic:mainfrom
efd6:11809-aws_bedrock
Nov 28, 2024
Merged

aws_bedrock: retain contextualGroundingPolicy check details#11890
efd6 merged 0 commit intoelastic:mainfrom
efd6:11809-aws_bedrock

Conversation

@efd6
Copy link
Copy Markdown
Contributor

@efd6 efd6 commented Nov 27, 2024

Proposed commit message

See title.

Checklist

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all data streams collect metrics or logs.
  • I have added an entry to my package's changelog.yml file.
  • I have verified that Kibana version constraints are current according to guidelines.
  • I have verified that any added dashboard complies with Kibana's Dashboard good practices

Author's Checklist

  • [ ]

How to test this PR locally

Related issues

Screenshots

@efd6 efd6 added enhancement New feature or request Team:Security-Service Integrations Security Service Integrations team [elastic/security-service-integrations] Integration:aws_bedrock Amazon Bedrock labels Nov 27, 2024
@efd6 efd6 self-assigned this Nov 27, 2024
@efd6 efd6 force-pushed the 11809-aws_bedrock branch from 42c9b10 to be801ec Compare November 27, 2024 08:23
@efd6 efd6 marked this pull request as ready for review November 27, 2024 08:55
@efd6 efd6 requested a review from a team as a code owner November 27, 2024 08:55
@elasticmachine
Copy link
Copy Markdown

Pinging @elastic/security-service-integrations (Team:Security-Service Integrations)

Copy link
Copy Markdown
Contributor

@shashank-elastic shashank-elastic left a comment

Choose a reason for hiding this comment

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

Filed mapping correction looks good

Copy link
Copy Markdown
Contributor

@chrisberkhout chrisberkhout left a comment

Choose a reason for hiding this comment

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

It looks like we should be looking at body.trace.guardrail.outputAssessments in general, since we are looking at body.amazon_bedrock_trace.guardrail.outputs. I probably should have done that in my last change, which started looking at outputs.

If that's added, then the other change is to expand which details objects get added to gen_ai.policy.match_detail.

What do you think about this version? The test output is slightly different.

--- a/packages/aws_bedrock/data_stream/invocation/elasticsearch/ingest_pipeline/default.yml
+++ b/packages/aws_bedrock/data_stream/invocation/elasticsearch/ingest_pipeline/default.yml
@@ -170,6 +170,7 @@ processors:
         "topicPolicy": "topic_policy"
         "customWords": "custom_words"
         "sensitiveInformationPolicy": "sensitive_information_policy"
+        "contextualGroundingPolicy": "contextual_grounding_policy"
         "invocationMetrics": "invocation_metrics"
         "guardrailCoverage": "guardrail_coverage"
         "textCharacters": "text_characters"
@@ -356,6 +357,7 @@ processors:
           )
           .flatMap(body -> [
             [body.trace?.guardrail?.inputAssessment],
+            body.trace?.guardrail?.outputAssessments,
             [body.amazon_bedrock_trace?.guardrail?.input],
             body.amazon_bedrock_trace?.guardrail?.outputs
           ].stream())
@@ -404,7 +406,7 @@ processors:
           .collect(Collectors.toList());
 
         ctx.gen_ai.policy.match_detail = nameAndDetailsOfPolicies.stream()
-          .filter(policy -> policy.details.match != null)
+          .filter(policy -> policy.details.match != null || policy.name == 'contextual_grounding_policy')
           .map(policy -> policy.details)
           .distinct()  // no sort because HashMap isn't Comparable
           .collect(Collectors.toList());

@efd6 efd6 force-pushed the 11809-aws_bedrock branch from be801ec to 5525432 Compare November 27, 2024 21:30
@efd6
Copy link
Copy Markdown
Contributor Author

efd6 commented Nov 27, 2024

Yeah, I think that's good. The changes that I see in the test expectation actually fix concerns that I had with the current behaviour, so that is nice. Will wait for @shashank-elastic's input though.

@elastic-vault-github-plugin-prod
Copy link
Copy Markdown

🚀 Benchmarks report

To see the full report comment with /test benchmark fullreport

@shashank-elastic
Copy link
Copy Markdown
Contributor

Yeah, I think that's good. The changes that I see in the test expectation actually fix concerns that I had with the current behaviour, so that is nice. Will wait for @shashank-elastic's input though.

@efd6 The filed mapping for outputassesments for contextualGroundingPolicy look good!

Comment on lines 408 to 422
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

My diff was on main version of this file, so this part would just be:

Suggested change
ctx.gen_ai.policy.match_detail = Stream.concat(
nameAndDetailsOfPolicies.stream()
.filter(policy -> policy.details.match != null || policy.name == 'contextual_grounding_policy')
.map(policy -> policy.details),
allBodies.stream()
.filter(body -> body.trace?.guardrail?.outputAssessments instanceof List)
.flatMap(body -> body.trace.guardrail.outputAssessments.stream())
.filter(output -> output instanceof HashMap)
.flatMap(output -> output.entrySet().stream())
.map(assessment -> assessment.getValue())
.filter(details -> details.contextualGroundingPolicy?.filters instanceof List)
.flatMap(details -> details.contextualGroundingPolicy.filters.stream())
)
.distinct() // no sort because HashMap isn't Comparable
.collect(Collectors.toList());
ctx.gen_ai.policy.match_detail = nameAndDetailsOfPolicies.stream()
.filter(policy -> policy.details.match != null || policy.name == 'contextual_grounding_policy')
.map(policy -> policy.details)
.distinct() // no sort because HashMap isn't Comparable
.collect(Collectors.toList());

Currently the 2nd part of the concat generates nothing because it uses the camel case name contextualGroundingPolicy.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for clarifying, I missed that. Much nicer.

@efd6 efd6 requested a review from chrisberkhout November 28, 2024 10:15
@elasticmachine
Copy link
Copy Markdown

💚 Build Succeeded

History

  • 💚 Build #18811 succeeded 687d504f666b086695ea82273f4c12baea22db59
  • 💚 Build #18755 succeeded be801ece6d962ed832afe3264a95f4713cbc912c

cc @efd6

@elastic-sonarqube
Copy link
Copy Markdown

@efd6 efd6 merged commit 072343f into elastic:main Nov 28, 2024
@elastic-vault-github-plugin-prod
Copy link
Copy Markdown

Package aws_bedrock - 0.15.0 containing this change is available at https://epr.elastic.co/package/aws_bedrock/0.15.0/

harnish-crest-data pushed a commit to chavdaharnish/integrations that referenced this pull request Feb 4, 2025
harnish-crest-data pushed a commit to chavdaharnish/integrations that referenced this pull request Feb 5, 2025
@efd6 efd6 deleted the 11809-aws_bedrock branch February 5, 2025 21:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request Integration:aws_bedrock Amazon Bedrock Team:Security-Service Integrations Security Service Integrations team [elastic/security-service-integrations]

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Amazon Bedrock]: Amazon Bedrock Guardrail Contextual grounding check is not mapped.

4 participants