(chore)APM: Refactor Bedrock Integration by yahya-mouman · Pull Request #5137 · DataDog/dd-trace-js

yahya-mouman · 2025-01-20T16:53:52Z

What does this PR do?

This PR adds the following changes to the AWS Bedrock APM Integration

Adds parsing for more variations of the model id in bedrock
Channel suffix space removal
Plugin rename from bedrock to bedrockruntime
Structure refactor adding a utils file with all the parsing logic

Please also review #5116
As it contains some fixture refactors to the testing code

Motivation

The addition of the LLMObs plugin for this integration makes it necessary to refactor the code and share utilities to avoid code duplication.

Plugin Checklist

Additional Notes

github-actions · 2025-01-20T16:54:34Z

Overall package size

Self size: 8.53 MB
Deduped: 94.89 MB
No deduping: 95.4 MB

Dependency sizes

| name | version | self size | total size | |------|---------|-----------|------------| | @datadog/libdatadog | 0.4.0 | 29.44 MB | 29.44 MB | | @datadog/native-appsec | 8.4.0 | 19.25 MB | 19.26 MB | | @datadog/native-iast-taint-tracking | 3.2.0 | 13.9 MB | 13.91 MB | | @datadog/pprof | 5.4.1 | 9.76 MB | 10.13 MB | | protobufjs | 7.2.5 | 2.77 MB | 5.16 MB | | @datadog/native-iast-rewriter | 2.6.1 | 2.59 MB | 2.73 MB | | @opentelemetry/core | 1.14.0 | 872.87 kB | 1.47 MB | | @datadog/native-metrics | 3.1.0 | 1.06 MB | 1.46 MB | | @opentelemetry/api | 1.8.0 | 1.21 MB | 1.21 MB | | import-in-the-middle | 1.11.2 | 112.74 kB | 826.22 kB | | source-map | 0.7.4 | 226 kB | 226 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | lru-cache | 7.18.3 | 133.92 kB | 133.92 kB | | pprof-format | 2.1.0 | 111.69 kB | 111.69 kB | | @datadog/sketches-js | 2.1.0 | 109.9 kB | 109.9 kB | | semver | 7.6.3 | 95.82 kB | 95.82 kB | | lodash.sortby | 4.7.0 | 75.76 kB | 75.76 kB | | ignore | 5.3.1 | 51.46 kB | 51.46 kB | | shell-quote | 1.8.1 | 44.96 kB | 44.96 kB | | istanbul-lib-coverage | 3.2.0 | 29.34 kB | 29.34 kB | | rfdc | 1.3.1 | 25.21 kB | 25.21 kB | | @isaacs/ttlcache | 1.4.1 | 25.2 kB | 25.2 kB | | tlhunter-sorted-set | 0.1.0 | 24.94 kB | 24.94 kB | | limiter | 1.1.5 | 23.17 kB | 23.17 kB | | dc-polyfill | 0.1.4 | 23.1 kB | 23.1 kB | | retry | 0.13.1 | 18.85 kB | 18.85 kB | | jest-docblock | 29.7.0 | 8.99 kB | 12.76 kB | | crypto-randomuuid | 1.0.0 | 11.18 kB | 11.18 kB | | ttl-set | 1.0.0 | 4.61 kB | 9.69 kB | | path-to-regexp | 0.1.12 | 6.6 kB | 6.6 kB | | koalas | 1.0.2 | 6.47 kB | 6.47 kB | | module-details-from-path | 1.0.3 | 4.47 kB | 4.47 kB |

_{🤖 This report was automatically generated by heaviest-objects-in-the-universe}

pr-commenter · 2025-01-20T17:03:32Z

Benchmarks

Benchmark execution time: 2025-01-24 12:51:37

Comparing candidate commit 255f8e6 in PR branch yahya/refactor-apm-bedrock-runtime with baseline commit 30efc06 in branch master.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 906 metrics, 22 unstable metrics.

sabrenner

just a couple comments - this is a really nice cleanup, awesome job!

…dex.js Co-authored-by: Sam Brenner <106700075+sabrenner@users.noreply.github.com>

…acing.js Co-authored-by: Sam Brenner <106700075+sabrenner@users.noreply.github.com>

…ils.js Co-authored-by: Sam Brenner <106700075+sabrenner@users.noreply.github.com>

…me' into yahya/refactor-apm-bedrock-runtime

Co-authored-by: Thomas Hunter II <tlhunter@datadog.com>

* refactor apm tracing * Update packages/datadog-plugin-aws-sdk/src/services/bedrockruntime/index.js Co-authored-by: Sam Brenner <106700075+sabrenner@users.noreply.github.com> * Update packages/datadog-plugin-aws-sdk/src/services/bedrockruntime/tracing.js Co-authored-by: Sam Brenner <106700075+sabrenner@users.noreply.github.com> * Update packages/datadog-plugin-aws-sdk/src/services/bedrockruntime/tracing.js Co-authored-by: Sam Brenner <106700075+sabrenner@users.noreply.github.com> * Update packages/datadog-plugin-aws-sdk/src/services/bedrockruntime/utils.js Co-authored-by: Sam Brenner <106700075+sabrenner@users.noreply.github.com> * remove shouldSetChoiceId override * remove shouldSetChoiceId override * lint * Update packages/datadog-instrumentations/src/aws-sdk.js Co-authored-by: Thomas Hunter II <tlhunter@datadog.com> --------- Co-authored-by: Sam Brenner <106700075+sabrenner@users.noreply.github.com> Co-authored-by: Thomas Hunter II <tlhunter@datadog.com>

tlhunter · 2025-02-14T20:30:48Z


 function getChannelSuffix (name) {
+  // some resource identifiers have spaces between ex: bedrock runtime
+  name = name.replaceAll(' ', '')


~~It looks like name can be undefined which causes this line to throw an error.~~

Actually I don't know this for sure. It could be that the user has mocked something via sinon incompletely.

- see #5137

refactor apm tracing

98f313c

yahya-mouman marked this pull request as ready for review January 20, 2025 17:03

yahya-mouman requested review from a team as code owners January 20, 2025 17:03

yahya-mouman changed the title ~~refactor apm tracing~~ [APM] Refactor Bedrock Integration Jan 20, 2025

yahya-mouman changed the title ~~[APM] Refactor Bedrock Integration~~ (Chore) APM : Refactor Bedrock Integration Jan 21, 2025

yahya-mouman changed the title ~~(Chore) APM : Refactor Bedrock Integration~~ (Chore)APM: Refactor Bedrock Integration Jan 21, 2025

sabrenner added the semver-patch label Jan 21, 2025

sabrenner reviewed Jan 21, 2025

View reviewed changes

yahya-mouman and others added 8 commits January 21, 2025 15:15

Update packages/datadog-plugin-aws-sdk/src/services/bedrockruntime/in…

464d44c

…dex.js Co-authored-by: Sam Brenner <106700075+sabrenner@users.noreply.github.com>

Update packages/datadog-plugin-aws-sdk/src/services/bedrockruntime/tr…

b797d55

…acing.js Co-authored-by: Sam Brenner <106700075+sabrenner@users.noreply.github.com>

Update packages/datadog-plugin-aws-sdk/src/services/bedrockruntime/tr…

0a3fe02

…acing.js Co-authored-by: Sam Brenner <106700075+sabrenner@users.noreply.github.com>

Update packages/datadog-plugin-aws-sdk/src/services/bedrockruntime/ut…

d209e07

…ils.js Co-authored-by: Sam Brenner <106700075+sabrenner@users.noreply.github.com>

CODEOWNERS

01bacb0

Merge remote-tracking branch 'origin/yahya/refactor-apm-bedrock-runti…

94fc104

…me' into yahya/refactor-apm-bedrock-runtime

remove shouldSetChoiceId override

c421371

remove shouldSetChoiceId override

dd3dca4

yahya-mouman changed the title ~~(Chore)APM: Refactor Bedrock Integration~~ (chore)APM: Refactor Bedrock Integration Jan 21, 2025

lint

098b074

sabrenner previously approved these changes Jan 21, 2025

View reviewed changes

Merge branch 'master' into yahya/refactor-apm-bedrock-runtime

798a0d3

tlhunter reviewed Jan 23, 2025

View reviewed changes

Comment thread packages/datadog-instrumentations/src/aws-sdk.js Outdated

tlhunter previously approved these changes Jan 23, 2025

View reviewed changes

Update packages/datadog-instrumentations/src/aws-sdk.js

255f8e6

Co-authored-by: Thomas Hunter II <tlhunter@datadog.com>

yahya-mouman dismissed stale reviews from tlhunter and sabrenner via 255f8e6 January 24, 2025 12:40

yahya-mouman requested a review from sabrenner January 24, 2025 12:40

sabrenner approved these changes Jan 24, 2025

View reviewed changes

yahya-mouman merged commit f41f5f7 into master Jan 24, 2025

yahya-mouman deleted the yahya/refactor-apm-bedrock-runtime branch January 24, 2025 13:02

szegedi mentioned this pull request Jan 28, 2025

v5.33.0 proposal #5160

Merged

tlhunter reviewed Feb 14, 2025

View reviewed changes

tlhunter added a commit that referenced this pull request Feb 14, 2025

(chore) APM: Fix Bedrock Integration Refactor

3eb96e3

- see #5137

tlhunter mentioned this pull request Feb 14, 2025

(chore) APM: Fix Bedrock Integration Refactor #5278

Merged

6 tasks

tlhunter added a commit that referenced this pull request Feb 19, 2025

(chore) APM: Fix Bedrock Integration Refactor (#5278)

00043df

- see #5137

sabrenner pushed a commit that referenced this pull request Feb 24, 2025

(chore) APM: Fix Bedrock Integration Refactor (#5278)

b191051

- see #5137

sabrenner pushed a commit that referenced this pull request Feb 24, 2025

(chore) APM: Fix Bedrock Integration Refactor (#5278)

8579c99

- see #5137

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(chore)APM: Refactor Bedrock Integration#5137

(chore)APM: Refactor Bedrock Integration#5137
yahya-mouman merged 12 commits intomasterfrom
yahya/refactor-apm-bedrock-runtime

yahya-mouman commented Jan 20, 2025 •

edited

Loading

Uh oh!

github-actions Bot commented Jan 20, 2025 •

edited

Loading

Uh oh!

pr-commenter Bot commented Jan 20, 2025 •

edited

Loading

Uh oh!

sabrenner left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tlhunter Feb 14, 2025 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

yahya-mouman commented Jan 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Motivation

Plugin Checklist

Additional Notes

Uh oh!

github-actions Bot commented Jan 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overall package size

Uh oh!

pr-commenter Bot commented Jan 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmarks

Uh oh!

sabrenner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tlhunter Feb 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

yahya-mouman commented Jan 20, 2025 •

edited

Loading

github-actions Bot commented Jan 20, 2025 •

edited

Loading

pr-commenter Bot commented Jan 20, 2025 •

edited

Loading

tlhunter Feb 14, 2025 •

edited

Loading