Skip to content

OTel: Allow users to add metrics exporters.#4189

Merged
dsessler7 merged 2 commits into
CrunchyData:mainfrom
dsessler7:otel-metrics-exporters
Jun 4, 2025
Merged

OTel: Allow users to add metrics exporters.#4189
dsessler7 merged 2 commits into
CrunchyData:mainfrom
dsessler7:otel-metrics-exporters

Conversation

@dsessler7

@dsessler7 dsessler7 commented May 30, 2025

Copy link
Copy Markdown
Collaborator

Checklist:

  • Have you added an explanation of what your changes do and why you'd like them to be included?
  • Have you updated or added documentation for the change, as applicable?
  • Have you tested your changes on all related environments with successful results, as applicable?
    • Have you added automated tests?

Type of Changes:

  • New feature
  • Bug fix
  • Documentation
  • Testing enhancement
  • Other

What is the current behavior (link to any open issues here)?

Currently, OTel metrics are only exported by the default prometheus exporter that we set up to work with our monitoring stack. Users cannot add their own metrics exporters.

What is the new behavior (if this is a feature change)?

  • Breaking change (fix or feature that would cause existing functionality to change)

Users can now add their own metrics exporters. The default prometheus exporter has a new component ID of prometheus/cpk-monitoring to differentiate it from any other prometheus exporters that users might add.

Other Information:


// If there are exporters to be added to the metrics pipelines defined
// in the spec, add them to the pipeline.
exporters := []ComponentID{Prometheus}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So there's no way for a user to remove the Prometheus exporter? (Of course, if they did, we'd maybe also want to change the way we expose the port?)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

No, but I'm not opposed to providing a way to remove it. I'm just not sure that we should remove it by default whenever the user provides their own exporters.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I keep thinking about the way we deal with debug for logs as a model, but maybe that's too different?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yeah feels pretty different to me. The debug exporter is there just so that there is some way to view logs if the user doesn't provide another solution, but the Prometheus exporter is key for integrating into our monitoring stack.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah, agreed -- we can use this PR to add other exporters and maybe another to remove Prometheus/remove the exposed port (if we want to. I can imagine someone who wants their own exporter might not want Prometheus at all).

(Though also I'm still curious if there's other ports to expose or other changes for other metrics exporters, but at least we've covered the major ones, I think.)

@benjaminjb

Copy link
Copy Markdown
Contributor

Wonder if we could add a metrics exporter stage to the Kuttl test?

@dsessler7

Copy link
Copy Markdown
Collaborator Author

Wonder if we could add a metrics exporter stage to the Kuttl test?

Just did. Please take a look.

@dsessler7 dsessler7 requested a review from benjaminjb June 4, 2025 07:04
Comment on lines +557 to +559
// The queries aren't really needed for this test and sheer number of queries
// would make this file excessively long (and string formatting presented it's
// own formatting headaches), so I am removing them

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

💯 for the comment and logic

@dsessler7 dsessler7 merged commit d6800dd into CrunchyData:main Jun 4, 2025
19 checks passed
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.

2 participants