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

Control if metrics or traces are sent to collector #98

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

ryanrolds
Copy link
Contributor

We currently only want to send metrics to our OTEL Collector. I'm seeing errors about not being able to send traces because the collector doesn't have tracing configured.

This PR adds a flags that allow us to turn off the sending of traces (or metrics if that is your thing).

@mdelapenya
Copy link
Owner

Before starting the review of this PR, I'd like to put the demos back (elastic, jaeger, etc) to work. I followed the instructions in the readme without success 🤔

@ryanrolds
Copy link
Contributor Author

Are the issues with the demo related to this PR? I don't think I broke anything. It's fine if they are not, just wanting clarity.

@mdelapenya
Copy link
Owner

No no, not related at all. Sorry for the confusion. I added that to express that we are adding code with the demos being broken.

@ryanrolds
Copy link
Contributor Author

In between some tasks, I will go through each of the demos and adjust them.

@mdelapenya
Copy link
Owner

I'm on PTO until the new year, so will go back to this right after that.

Thank you so much for your work 🙇

@mdelapenya mdelapenya self-assigned this Jan 2, 2025
@mdelapenya mdelapenya added the enhancement New feature or request label Jan 2, 2025
Copy link
Owner

@mdelapenya mdelapenya left a comment

Choose a reason for hiding this comment

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

LGTM, although I'm missing a test for this, probably because the current state of the code does not allow it.

Can we add a TODO comment, or simply better opening an issue?, so that we can refactor the initialisation of the otel providers and eventually write tests about that?

@mdelapenya
Copy link
Owner

OR, if you find it testable, e.g. adding multiple instances of the TestMain function with different flags set and expected outcomes, please add it 🙏 I can work on generalise that test function in a separate PR if you see it appropriate

@ryanrolds
Copy link
Contributor Author

i will see about generalizing things about.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants