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

Moved common processor files and updated refs #1574

Merged

Conversation

joe-elliott
Copy link
Contributor

@joe-elliott joe-elliott commented Aug 18, 2020

Description:

  • Moved attribute processor config files to the processor/processorhelper folder to allow for external use
  • Updated references
  • Improved code coverage

There are obviously a lot of ways to expose these files for external use. I chose the common go convention of using a pkg folder, but otherwise kept the folder structure. I am, however, fine putting these file wherever the maintainers would prefer. Please let me know if you'd like them in a different location.
Per discussions below processor/processorhelper was agreed upon.

Link to tracking Issue:
Fixes #1411

Testing:
n/a

Documentation:
n/a

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Aug 18, 2020

CLA Check
The committers are authorized under a signed CLA.

@project-bot project-bot bot added this to In progress in Collector Aug 18, 2020
@codecov
Copy link

codecov bot commented Aug 18, 2020

Codecov Report

Merging #1574 into master will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1574      +/-   ##
==========================================
+ Coverage   91.91%   91.94%   +0.02%     
==========================================
  Files         255      255              
  Lines       17546    17546              
==========================================
+ Hits        16128    16132       +4     
+ Misses       1013     1011       -2     
+ Partials      405      403       -2     
Impacted Files Coverage Δ
processor/processorhelper/attraction.go 100.00% <ø> (ø)
processor/processorhelper/hasher.go 100.00% <ø> (ø)
processor/resourceprocessor/resource_processor.go 100.00% <ø> (ø)
processor/attributesprocessor/attributes.go 94.11% <100.00%> (ø)
processor/attributesprocessor/factory.go 86.20% <100.00%> (ø)
processor/resourceprocessor/factory.go 100.00% <100.00%> (ø)
translator/internaldata/resource_to_oc.go 91.48% <0.00%> (+2.12%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2acc904...a523428. Read the comment docs.

@joe-elliott joe-elliott changed the title Moved pkg files and updated refs Moved common processor files and updated refs Aug 18, 2020
Copy link
Member

@bogdandrutu bogdandrutu left a comment

Choose a reason for hiding this comment

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

We don't follow the PKG convention. So probably filters, helpers are better names.

@joe-elliott
Copy link
Contributor Author

There is already a /processor/processorhelper package. Would that make sense?

@joe-elliott
Copy link
Contributor Author

Changes made, but CI is failing. After investigating I don't believe the failures are related to any of the changes I have made.

@bogdandrutu
Copy link
Member

Please rebase, the CI for windows-msi was changed

@bogdandrutu
Copy link
Member

Also please update description :)

Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
@joe-elliott
Copy link
Contributor Author

Thanks for the heads up @bogdandrutu. Tests passing.

Collector automation moved this from In progress to Reviewer approved Aug 19, 2020
@bogdandrutu bogdandrutu merged commit cadd498 into open-telemetry:master Aug 19, 2020
Collector automation moved this from Reviewer approved to Done Aug 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Collector
  
Done
Development

Successfully merging this pull request may close these issues.

Attribute processor cannot be programmatically configued
2 participants