-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[apm] peer.service aggregation for trace stats, option to compute stats based on span.kind #16103
Merged
Merged
Changes from 47 commits
Commits
Show all changes
53 commits
Select commit
Hold shift + click to select a range
df3e63f
[apm] initial commit; add peer.service to stats
jdgumz 8cda026
[apm] re-generate msgpack to account for peer.service
jdgumz d0e1321
[apm] add tests for peer.service aggregation
jdgumz f940919
[apm] add comment to explain peer_service in proto
jdgumz fb95f5c
[apm] comment revision
jdgumz 2ad214f
[apm] add release notes
jdgumz af0f5f6
Update pkg/trace/stats/aggregation.go
jdgumz bfaa1e2
Update pkg/trace/stats/aggregation.go
jdgumz 1593a31
Merge branch 'apm/peer-service-aggregation-for-trace-stats' of github…
jdgumz 27c1a02
[apm] add consideration of span.kind == INTERNAL for calculating stats
jdgumz 1e74f07
[apm] consider CLIENT/PRODUCER specifically rather than INTERNAL for …
jdgumz 2e80bb4
[apm] remove incorrect comment
jdgumz 0353e82
[apm] correct tests, update release notes
jdgumz 4eb1a0a
[apm] RemoteOutbound -> RemoteOutgoing
jdgumz dcfe044
[apm] remove unnecessary newlines
jdgumz a776a46
[apm] update peer.service test to use a more realistic example for it…
jdgumz b0a39d6
Update pkg/trace/stats/concentrator.go
jdgumz 59129de
Update releasenotes/notes/add-peer-service-for-trace-stats-225f1b90c9…
jdgumz 66f690d
Update releasenotes/notes/add-peer-service-for-trace-stats-225f1b90c9…
jdgumz a405615
[apm] fix bug from remote commit
jdgumz ba3ad76
[apm] remove consideration of span.kind for trace stats computation
jdgumz 588a0ec
[apm] add normalization for peer.service
jdgumz 2586730
[apm] add concentrator configuration to enable/disable peer.service s…
jdgumz e7222b9
[apm] ensure peer.service is exported from client stats aggregator as…
jdgumz 11cfffb
[apm] update config_template.yaml
jdgumz 90c0b09
[apm] further clarify effect of disabling peer.service stats aggregation
jdgumz 45758c6
[apm] fix test failures
jdgumz c7c7e51
[apm] rework configuration of peer.service, add back extra aggregator…
jdgumz 394c33b
[apm] set peer service aggregation to false by default, ensure config…
jdgumz 4fa25f1
[apm] revise variable name to be consistent, add check for peer servi…
jdgumz 0986d4f
[apm] add logic to compute stats based on specific span.kind values
jdgumz 2e68665
[apm] add config for option to enable stats computation by span.kind
jdgumz 2c1f694
[apm] update documentation and release notes
jdgumz 0cf26cc
Update cmd/trace-agent/config/config.go
jdgumz 63c572a
Update pkg/trace/traceutil/span.go
jdgumz 6dc94ad
[apm] rename peer service aggregation config field
jdgumz ffd0975
[apm] add more test cases to ensure case insensitivity for span.kind …
jdgumz cee986d
[apm] move span.kind check func implementation to concentrator.go
jdgumz c4ea367
Update pkg/trace/stats/concentrator.go
jdgumz d6a545c
[apm] finish move of span.kind check func
jdgumz 1a07963
[apm] go back to setting a bool flag for peer.service in concentrator
jdgumz 1454017
[apm] add back ExtraAggregators for info test case
jdgumz 76957f4
[apm] fix testutil fixture BucketWithSpans
jdgumz 1f8c1e3
[apm] move peer.service aggregation check back into NewAggregationFro…
jdgumz 01156fc
[apm] change CSA to use bool for peer svc aggregation as well
jdgumz 98136a2
[apm] remove unused const
jdgumz ecdeaa4
[apm] update guidance on new config options
jdgumz c65f007
[apm] add small test for bucket aggregation key creation and peer.ser…
jdgumz b4bdba1
[apm] remove unused field
jdgumz 4450bb5
[apm] fix fuzz test
jdgumz 29f9320
[apm] fix stats info tests
jdgumz 94886e0
Merge branch 'main' into apm/peer-service-aggregation-for-trace-stats
jdgumz 8ad14ae
Merge branch 'main' into apm/peer-service-aggregation-for-trace-stats
jdgumz File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
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
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
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
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should
ExtraAggregators
get marked as deprecated as opposed to deleted? My worry here is that it might still live in a customer configuration somewhere, and if we were to re-introduce it in the future we could run into problems. I think this has to be considered like a "reserved" field of sorts.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternatively, we could start using is again with
peer.service
as a possible value that we set if enabled.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added this back with a comment about it being deprecated. It's not actually used in the current agent code because we don't create aggregation keys with strings anymore. We have the new struct key format, so we can't arbitrarily aggregate over other tag keys.
I think this only works for much older versions of the agent (probably any agent that doesn't use the new ClientStatsPayloads).