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

Enable custom attributes for events #187

Merged
merged 7 commits into from
Nov 29, 2019
Merged

Enable custom attributes for events #187

merged 7 commits into from
Nov 29, 2019

Conversation

alejandrodnm
Copy link
Contributor

Description of the changes

This PR allows events with attributes to be decorated with the entity customAttributes when they are added to via the entity.AddEvent function.

PR Review Checklist

Author

  • [x ] add a risk label after carefully considering the "blast radius" of your changes
  • [x ] describe the intent of your changes in the description. don't just rewrite your code in prose
  • [x ] assign at least one reviewer
  • [x ] document feature

Reviewer

  • review code for readability
  • verify that high risk behavior changes are well tested
  • check license for any new external dependency
  • request documenation about anything that isn't clear and obvious
  • document agent version supporting the feature

@alejandrodnm alejandrodnm requested a review from a team August 26, 2019 16:44
varas
varas reviewed Sep 19, 2019
View reviewed changes
integration/entity_test.go Outdated Show resolved Hide resolved
varas
varas reviewed Sep 19, 2019
View reviewed changes
integration/entity_test.go Outdated Show resolved Hide resolved
varas
varas reviewed Sep 19, 2019
View reviewed changes
data/metric/metrics.go Outdated Show resolved Hide resolved
varas
varas suggested changes Sep 19, 2019
View reviewed changes
Copy link
Contributor

@varas varas left a comment

Choose a reason for hiding this comment

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

Thanks for this! I added some questions to be answered.
Also, would you mind to add some documentation bundling its use case/s?

@alejandrodnm alejandrodnm force-pushed the adn/default_attributes_for_events branch from bb1ff98 to 9733cd8 Compare November 9, 2019 11:10
@alejandrodnm alejandrodnm requested review from a team and varas November 9, 2019 11:10
@alejandrodnm alejandrodnm force-pushed the adn/default_attributes_for_events branch from 9733cd8 to 21bf89f Compare November 12, 2019 09:20
mariomac
mariomac previously approved these changes Nov 12, 2019
View reviewed changes
@mariomac
Copy link
Contributor

LGTM as long as Juan's comments are addressed

@alejandrodnm alejandrodnm reopened this Nov 12, 2019
varas
varas reviewed Nov 25, 2019
View reviewed changes
data/attribute/attribute.go
@@ -0,0 +1,46 @@
package attribute

Copy link
Contributor

Choose a reason for hiding this comment

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

#187 (comment)

varas
varas suggested changes Nov 25, 2019
View reviewed changes
Copy link
Contributor

@varas varas left a comment

Choose a reason for hiding this comment

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

remove attribute package unless there's a reason for it

@alejandrodnm
Copy link
Contributor Author

@varas sorry i missed one of your comments, I replied here #187 (comment)

@alejandrodnm alejandrodnm reopened this Nov 25, 2019
@varas varas merged commit 23e2c90 into master Nov 29, 2019
@varas varas deleted the adn/default_attributes_for_events branch November 29, 2019 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@varas varas varas approved these changes

@mariomac mariomac mariomac left review comments

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

Successfully merging this pull request may close these issues.
3 participants
@alejandrodnm @mariomac @varas