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 singleton flags to factories and standardize attributes #12057

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

djaglowski
Copy link
Member

This PR is an extension of #11344 that addresses #11814.

Major changes:

  • Add a Metadata() method to receiver.Factory, exporter.Factory and connector.Factory (processor.Factory excluded intentionally as described in Loggers for shared components should not report signal type. #11814). These Metadata() methods return Metadata structs, which act similarly to consumer.Capabilities in that they are intended as a set of information which describes the components which the factory produces.
  • Remodels each component's as a proper go.opentelemetry.io/otel/attribute.Set. This set is used as the ID in the graph, but also as the set of attributes that creates the logger. In the future, the set will be automatically used to describe other aspects of the collector's own telemetry.
  • The logger fields are changed to match those described in the component telemetry RFC.

Minor changes:

  • Unknown component types are detected earlier in the startup process. (When the graph is being assembled, rather than when the components of the graph are being instantiated.) This is necessary so that we can obtain the Metadata from the appropriate factory. There is a minor user facing change in that the error message is not wrapped in a "could not build" prefix. I moved the processors to this pattern as well in order to match the other components.

@djaglowski djaglowski requested review from mx-psi, dmathieu and a team as code owners January 8, 2025 19:47
@djaglowski djaglowski changed the title Singleton flags and attributes Add singleton flags to factories and standardize attributes Jan 8, 2025
@djaglowski djaglowski marked this pull request as draft January 8, 2025 19:47
@djaglowski
Copy link
Member Author

If desired, I believe I can split this PR into several smaller PRs. However, I wanted to put this version up in order to show the overall picture.

@djaglowski djaglowski force-pushed the singleton-flags-and-attributes branch 2 times, most recently from 0717a5d to 79e1bcb Compare January 8, 2025 20:00
Copy link

codecov bot commented Jan 8, 2025

Codecov Report

Attention: Patch coverage is 98.64407% with 4 lines in your changes missing coverage. Please review.

Project coverage is 91.72%. Comparing base (7425fe8) to head (256d7c9).

Files with missing lines Patch % Lines
service/internal/graph/exporter.go 92.59% 2 Missing ⚠️
service/internal/graph/receiver.go 95.23% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #12057      +/-   ##
==========================================
+ Coverage   91.67%   91.72%   +0.05%     
==========================================
  Files         455      456       +1     
  Lines       24010    24171     +161     
==========================================
+ Hits        22010    22171     +161     
  Misses       1629     1629              
  Partials      371      371              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@djaglowski
Copy link
Member Author

djaglowski commented Jan 10, 2025

I discovered when adding tests that singleton connectors will introduce broader changes into the way we build the graph, since we're wrapping each instance as a specific type of consumer. I've not been able to get it working yet but either way, the additional complexity seems not worth for now. I believe there are some reasons why singleton connectors would be useful but I'm going to propose for now that singleton instances are supported only for receivers and exporters. In the future we can try to add singleton connectors.

@djaglowski djaglowski force-pushed the singleton-flags-and-attributes branch 5 times, most recently from 32b1ba7 to 90a4238 Compare January 10, 2025 20:02
@djaglowski djaglowski force-pushed the singleton-flags-and-attributes branch from 90a4238 to 6a38f9c Compare January 10, 2025 20:49
@djaglowski djaglowski force-pushed the singleton-flags-and-attributes branch 2 times, most recently from bb19bc7 to 9fc517f Compare January 10, 2025 21:15
github-merge-queue bot pushed a commit that referenced this pull request Jan 10, 2025
This PR just cleans up some naming in the `testcomponents` package, so
that no component kind has a monopoly on the generic version of a
variable or function name.

Subset of #12057
@djaglowski djaglowski force-pushed the singleton-flags-and-attributes branch from 9fc517f to 7772955 Compare January 10, 2025 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant