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

[connector/servicegraph]Servergraph component does not merge different dimension metrics #35287 #35288

Conversation

hongweipeng
Copy link

Description:
Servergraph component does not merge different dimension metrics。

Link to tracking Issue:
#35287

Copy link
Contributor

@mapno mapno left a comment

Choose a reason for hiding this comment

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

LGTM. Would be great to have a test to verify this is now fixed.

@hongweipeng
Copy link
Author

Unit tests have been added.

@hongweipeng hongweipeng force-pushed the fix-servergraph-merge-diff-dimensions branch from 03d7499 to 765f0dd Compare September 24, 2024 05:50
connector/servicegraphconnector/connector_test.go Outdated Show resolved Hide resolved
continue
for _, kind := range []string{serverKind, clientKind} {
for _, dimName := range p.config.Dimensions {
if dim, ok := edgeDimensions[kind+"_"+dimName]; ok {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if dim, ok := edgeDimensions[kind+"_"+dimName]; ok {
if dim, ok := edgeDimensions[kind + "_" + dimName]; ok {

Nit: Whitespace around + operator.

Copy link
Author

Choose a reason for hiding this comment

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

There is no space in the original code, such as in the upsertDimensions function. I think it is better to keep the original style.

.chloggen/fix-servergraph-merge-diff-dimensions.yaml Outdated Show resolved Hide resolved
connector/servicegraphconnector/connector_test.go Outdated Show resolved Hide resolved
hongweipeng and others added 2 commits October 7, 2024 18:17
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Oct 22, 2024
Copy link
Contributor

github-actions bot commented Nov 5, 2024

Closed as inactive. Feel free to reopen if this PR is still being worked on.

@github-actions github-actions bot closed this Nov 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants