chore: add default ports to context.service.target.name for exit HTTP spans
#3590
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
the
agent-nodejs
david-luna
changed the title
serviceTarget.name context.service.target.name for exit HTTP spans
6 tasks
trentm
approved these changes
Aug 29, 2023
Comment on lines
+54
to
+56
| // UPDATE: there is a PR fixing it https://github.com/elastic/apm-agent-nodejs/pull/3589 | ||
| // making this test fail so for a short period we need to accept both cases | ||
| // before removing this tweak. |
There was a problem hiding this comment.
I'm a little confused. Should we take #3589 first and then this fix? Or perhaps add the lib/instrumentation/span.js changes as part of #3589? Then the shared test file change, and the APM agent changes to match it are in the same commit.
If you prefer to do them separately (this one first), then that's cool too.
There was a problem hiding this comment.
What I had in mind is 1st to add the default port and then let updatecli to update the JSON spec file in #3589
With this change in the conditional the test accepts both formats in the spec file (URL as object or as string) so the steps I had planned are:
- merge this PR
- merge [Automation] Update JSON specs #3589 when tests pass
- remove the
ifblock in a new PR (no longer needed)
I'll check if I can do the last stem on #3589 so we avoid an extra PR
david-luna
deleted the
dluna/chore-update-service-resource-inference-tests
branch
PeterEinberger
pushed a commit
to fpm-git/apm-agent-nodejs
that referenced
this pull request
Aug 20, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is conform with the agreements of the discussion in elastic/apm#700 which in summary says that default ports should be explicit in
serviceTarget.nameproperty of the span.Updatecli workflows are failing because of that and JSON specs cannot be updated #3589
Checklist