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.

Sign up for GitHub

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

Jump to bottom

fix: decorate events with custom_attributes #939

Merged
merged 2 commits into from
Jan 31, 2022
Merged

fix: decorate events with custom_attributes #939

merged 2 commits into from
Jan 31, 2022

Conversation

rubenruizdegauna
Copy link
Contributor

@rubenruizdegauna rubenruizdegauna commented Jan 26, 2022 •
edited
Loading

Decorate events with custom_attributes as it is done with metrics.

@kang-makes can you confirm the structure below is what you would expect?

Screenshot 2022-01-26 at 10 06 30

rubenruizdegauna
rubenruizdegauna commented Jan 26, 2022
View reviewed changes
internal/agent/status/status_test.go
return err == nil && res.StatusCode == 200
},
time.Second, 10*time.Millisecond)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

☝️ unrelated but flaky test. Wait until the server is ready.

kang-makes
kang-makes reviewed Jan 27, 2022
View reviewed changes
Copy link
Contributor

@kang-makes kang-makes left a comment

Choose a reason for hiding this comment

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

I sent a couple of events and it seems to be decorating correctly.

But I don't fully understand the changes made here. From my side, as a user of this agent, looks good.

pkg/integrations/v4/dm/emitter_no_register.go Show resolved Hide resolved
kang-makes
kang-makes previously approved these changes Jan 28, 2022
View reviewed changes
Copy link
Contributor

@kang-makes kang-makes left a comment

Choose a reason for hiding this comment

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

I was not testing this properly. Works perfectly.

Thanks for you fast answer :)

rogercoll
rogercoll reviewed Jan 28, 2022
View reviewed changes
Copy link
Contributor

@rogercoll rogercoll left a comment

Choose a reason for hiding this comment

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

LGTM

rogercoll
rogercoll previously approved these changes Jan 28, 2022
View reviewed changes
@rubenruizdegauna rubenruizdegauna force-pushed the decorate_events_with_custom_attributes branch from e2e6b1c to 3bd2300 Compare January 31, 2022 09:18
@rubenruizdegauna rubenruizdegauna merged commit 4f584a2 into master Jan 31, 2022
@rubenruizdegauna rubenruizdegauna deleted the decorate_events_with_custom_attributes branch January 31, 2022 09:52
@davidgit davidgit mentioned this pull request Feb 14, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rogercoll rogercoll rogercoll approved these changes

@kang-makes kang-makes kang-makes left review comments

@brushknight brushknight Awaiting requested review from brushknight

@cristianciutea cristianciutea Awaiting requested review from cristianciutea

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@rubenruizdegauna @rogercoll @kang-makes