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 entry point to build graph-based pipelines #7023

Merged
merged 1 commit into from
Jan 26, 2023

Conversation

djaglowski
Copy link
Member

Part of #6700

The remainder of the connectors implementation is concerned with setting up the component graph and building each node's associated component. I believe the best way to add these capabilities incrementally will be to so in a top-down way.

  • This PR adds the entry point to building the component graph.
  • Next, outline the steps to set up and build the graph.
  • Then add nodes, then edges.
  • Finally, build each node's associated component.

#6700 contains extensive testing of the graph as a whole. However, these tests interface primarily with the graph's built components. I'm not seeing a reasonable way to include coverage for a partially built graph, which I expect we would have if we follow the contribution path I've described above. My hope is that we can move forward for a few PRs based on the understanding that test coverage will be comprehensive in the end.


This PR adds the entry point to building the component graph. This entry point is hidden behind the same feature gate that was already to the otelcol package in #6964. However, it moves the gate to the service so that both packages can use it.

@djaglowski djaglowski added Skip Changelog PRs that do not require a CHANGELOG.md entry area:connector labels Jan 25, 2023
@djaglowski djaglowski force-pushed the connectors-entry-point branch from 702729b to 46be367 Compare January 25, 2023 15:27
@djaglowski djaglowski marked this pull request as ready for review January 25, 2023 16:36
@djaglowski djaglowski requested review from a team, mx-psi, jpkrohling and bogdandrutu January 25, 2023 16:36
service/service.go Outdated Show resolved Hide resolved
@djaglowski djaglowski force-pushed the connectors-entry-point branch from 46be367 to a322015 Compare January 25, 2023 18:01
@codecov
Copy link

codecov bot commented Jan 25, 2023

Codecov Report

Base: 90.33% // Head: 90.29% // Decreases project coverage by -0.05% ⚠️

Coverage data is based on head (7813479) compared to base (ef609c2).
Patch coverage: 40.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7023      +/-   ##
==========================================
- Coverage   90.33%   90.29%   -0.05%     
==========================================
  Files         245      245              
  Lines       14644    14654      +10     
==========================================
+ Hits        13229    13232       +3     
- Misses       1146     1152       +6     
- Partials      269      270       +1     
Impacted Files Coverage Δ
otelcol/config.go 96.38% <0.00%> (ø)
service/graph.go 71.42% <0.00%> (-8.58%) ⬇️
service/service.go 66.14% <60.00%> (-1.36%) ⬇️

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.

@bogdandrutu
Copy link
Member

Please rebase

@djaglowski djaglowski force-pushed the connectors-entry-point branch from a322015 to 7813479 Compare January 26, 2023 02:22
@djaglowski
Copy link
Member Author

Thanks, this has been rebased

@djaglowski
Copy link
Member Author

Can this be merged?

@dmitryax dmitryax merged commit 8d4b9d5 into open-telemetry:main Jan 26, 2023
@djaglowski djaglowski deleted the connectors-entry-point branch January 26, 2023 23:51
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.

5 participants