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

WIP - Demonstrate graph-based metrics #11311

Closed

Conversation

djaglowski
Copy link
Member

This is very incomplete but I want to show the basic idea of adding incoming/outgoing metrics at the graph level.

Obviously this code is pretty gnarly to begin with, and this only makes it worse, but this is superior to other approaches because:

  1. All attributes described in Attributes for component instancing #11179 are already available exactly where they are needed.
  2. Unlike Add 'pipeline' attribute to processor metrics #11171, changes are not needed elsewhere.
  3. Unlike Add ingoing and outgoing counts to processorhelper #10910, these metrics will apply to all processors, not just those built with processorhelper. This benefit translates to all component kinds too though.
  4. Additional metrics, such as proposed in [processorhelper] Add throughput metrics #11147 can be added quite easily.
  5. For processors and connectors, it's likely we could move span capture to this level.
  6. Future functionality such as proposed in Remotetap extension concept #10963 may be inserted in these consumer wrappers as well.

bogdandrutu pushed a commit that referenced this pull request Oct 1, 2024
Having spent some time on #11311, I think it may be time to start
refactoring this codebase into a more maintainable state. This PR just
moves the various types of nodes into separate files, which I think is a
bit more readable when considering changes.
Copy link
Member

@mx-psi mx-psi left a comment

Choose a reason for hiding this comment

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

I think this is the right approach to do this; it gives us more flexibility in terms of the API and we have the visibility we need at this level. I support this.

cc @open-telemetry/collector-maintainers PTAL so we can discuss the high-level approach before moving forward

jackgopack4 pushed a commit to jackgopack4/opentelemetry-collector that referenced this pull request Oct 8, 2024
Having spent some time on open-telemetry#11311, I think it may be time to start
refactoring this codebase into a more maintainable state. This PR just
moves the various types of nodes into separate files, which I think is a
bit more readable when considering changes.
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Oct 18, 2024
Copy link
Contributor

github-actions bot commented Nov 5, 2024

Closed as inactive. Feel free to reopen if this PR is still being worked on.

@github-actions github-actions bot closed this Nov 5, 2024
HongChenTW pushed a commit to HongChenTW/opentelemetry-collector that referenced this pull request Dec 19, 2024
Having spent some time on open-telemetry#11311, I think it may be time to start
refactoring this codebase into a more maintainable state. This PR just
moves the various types of nodes into separate files, which I think is a
bit more readable when considering changes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants