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

Add full component graph build #7045

Merged

Conversation

djaglowski
Copy link
Member

@djaglowski djaglowski commented Jan 27, 2023

This PR contains the bulk of what remains of #6700

It is a large PR, but mostly because of tests.

I'm presenting this as an alternative to the incremental top-down approach described in #7023. The main benefit of this would be to prevent a temporary lapse in test coverage.

@djaglowski djaglowski added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Jan 27, 2023
@codecov
Copy link

codecov bot commented Jan 27, 2023

Codecov Report

Base: 89.40% // Head: 90.58% // Increases project coverage by +1.18% 🎉

Coverage data is based on head (63976de) compared to base (84c3d00).
Patch coverage: 99.21% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7045      +/-   ##
==========================================
+ Coverage   89.40%   90.58%   +1.18%     
==========================================
  Files         293      293              
  Lines       13999    14240     +241     
==========================================
+ Hits        12516    12900     +384     
+ Misses       1215     1072     -143     
  Partials      268      268              
Impacted Files Coverage Δ
service/pipelines.go 93.66% <91.30%> (-0.28%) ⬇️
connector/connector.go 99.59% <100.00%> (+<0.01%) ⬆️
service/graph.go 98.95% <100.00%> (+27.52%) ⬆️
...rvice/internal/testcomponents/example_processor.go 100.00% <100.00%> (ø)
service/nodes.go 100.00% <100.00%> (+95.50%) ⬆️
...ervice/internal/proctelemetry/process_telemetry.go 82.53% <0.00%> (+10.74%) ⬆️
service/internal/components/loggers.go 100.00% <0.00%> (+24.00%) ⬆️
...rvice/internal/testcomponents/example_connector.go 100.00% <0.00%> (+100.00%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@djaglowski djaglowski force-pushed the connectors-full-comp-graph branch 3 times, most recently from 2aeba98 to 0b8d98d Compare January 27, 2023 14:35
@djaglowski djaglowski marked this pull request as ready for review January 27, 2023 15:13
@djaglowski djaglowski requested review from a team and jpkrohling January 27, 2023 15:13
service/nodes.go Outdated Show resolved Hide resolved
service/nodes.go Outdated Show resolved Hide resolved
service/nodes.go Outdated Show resolved Hide resolved
service/graph.go Outdated Show resolved Hide resolved
service/graph.go Outdated Show resolved Hide resolved
service/graph.go Show resolved Hide resolved
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.

I forgot to press submit review, I am sorry

service/graph.go Show resolved Hide resolved
service/graph.go Outdated Show resolved Hide resolved
service/graph.go Outdated Show resolved Hide resolved
service/graph.go Outdated Show resolved Hide resolved
service/graph.go Outdated Show resolved Hide resolved
service/graph.go Outdated Show resolved Hide resolved
service/graph.go Outdated Show resolved Hide resolved
service/graph.go Outdated Show resolved Hide resolved
service/graph.go Show resolved Hide resolved
service/graph.go Outdated Show resolved Hide resolved
service/graph.go Outdated Show resolved Hide resolved
service/graph.go Outdated Show resolved Hide resolved
service/graph.go Outdated Show resolved Hide resolved
@djaglowski djaglowski force-pushed the connectors-full-comp-graph branch 4 times, most recently from 66f648a to 6ab3baf Compare February 1, 2023 20:37
@djaglowski
Copy link
Member Author

I think I've addressed all feedback. Most items were addressed in separate commits for the sake of readability. I can squash when you think it's ready.

service/graph.go Show resolved Hide resolved
service/graph.go Outdated Show resolved Hide resolved
service/nodes.go Outdated Show resolved Hide resolved
service/nodes.go Outdated Show resolved Hide resolved
@djaglowski djaglowski force-pushed the connectors-full-comp-graph branch from 6ab3baf to fce05a8 Compare February 2, 2023 18:09
service/nodes.go Outdated Show resolved Hide resolved
@bogdandrutu bogdandrutu merged commit 490fd6a into open-telemetry:main Feb 3, 2023
@djaglowski djaglowski deleted the connectors-full-comp-graph branch February 3, 2023 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:connector Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants