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

Fix Upstream Service Mapping #418

Merged
merged 9 commits into from
Aug 24, 2023
Merged

Fix Upstream Service Mapping #418

merged 9 commits into from
Aug 24, 2023

Conversation

zARODz11z
Copy link
Contributor

@zARODz11z zARODz11z commented Aug 23, 2023

What does this PR do?

Fixes inferred spans service mapping by using service.name

Motivation

See SLS-4019 for more context, upstream service mapping not working in JS due to not using service.name

Testing Guidelines

Additional Notes

Types of Changes

  • Bug fix
  • New feature
  • Breaking change
  • Misc (docs, refactoring, dependency upgrade, etc.)

Check all that apply

  • This PR's description is comprehensive
  • This PR contains breaking changes that are documented in the description
  • This PR introduces new APIs or parameters that are documented and unlikely to change in the foreseeable future
  • This PR impacts documentation, and it has been updated (or a ticket has been logged)
  • This PR's changes are covered by the automated tests
  • This PR collects user input/sensitive content into Datadog
  • This PR passes the integration tests (ask a Datadog member to run the tests)

@@ -112,12 +112,12 @@ export class SpanInferrer {
endpoint: path,
resource_names: resourceName,
request_id: context?.awsRequestId,
Copy link
Contributor Author

@zARODz11z zARODz11z Aug 24, 2023

Choose a reason for hiding this comment

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

Via this branch I published v20 of dd lambda js to sandbox and asserted correct upstream service mapping for apigw and sns. The other services service.name are properly being added as shown by the spec file above and snapshots

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Screenshot 2023-08-23 at 5 09 50 PM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the issue why remapping wasnt working was the service.name is supported by backend and in the past it when it wasn't present it must of fallen back to the lambda function service name

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

SLS-4019

@zARODz11z zARODz11z changed the title debugging why mapping wont work in js Fix Upstream Service Mapping Aug 24, 2023
@zARODz11z zARODz11z marked this pull request as ready for review August 24, 2023 00:59
@zARODz11z zARODz11z requested a review from a team as a code owner August 24, 2023 00:59
@zARODz11z zARODz11z merged commit 5e4a429 into main Aug 24, 2023
15 checks passed
@zARODz11z zARODz11z deleted the ar/fixServiceMapping branch August 24, 2023 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants