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

No validations in otlphttpexporter #4709

Closed
anupamdalmia10 opened this issue Jan 19, 2022 · 6 comments · Fixed by #4860
Closed

No validations in otlphttpexporter #4709

anupamdalmia10 opened this issue Jan 19, 2022 · 6 comments · Fixed by #4860
Assignees
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@anupamdalmia10
Copy link
Contributor

anupamdalmia10 commented Jan 19, 2022

Describe the bug
A clear and concise description of what the bug is.

Validate method does not evaluate the given config for the exporter.

// Validate checks if the exporter configuration is valid

Steps to reproduce
If possible, provide a recipe for reproducing the error.

When collector is started with no value for endpoint in config.yaml for otlphttpexporter, collector crashes saying either endpoint or traces_endpoint (for traces pipeline) must be defined. Similar behaviour is observed for metrics and logs pipeline.

What did you expect to see?
A clear and concise description of what you expected to see.

These checks should be performed in Validate method, instead they are being done in factory.go in composeSignalURL method.

func composeSignalURL(oCfg *Config, signalOverrideURL string, signalName string) (string, error) {

What did you see instead?
A clear and concise description of what you saw instead.

What version did you use?
Version: (e.g., v0.4.0, 1eb551b, etc)

What config did you use?
Config: (e.g. the yaml config file)

image

Environment
OS: (e.g., "Ubuntu 20.04")
Compiler(if manually compiled): (e.g., "go 14.2")

Additional context
Add any other context about the problem here.

@anupamdalmia10 anupamdalmia10 added the bug Something isn't working label Jan 19, 2022
@jpkrohling jpkrohling added the good first issue Good for newcomers label Jan 19, 2022
@jpkrohling
Copy link
Member

Good catch. The OTLP exporter was done before we had Validate and never changed to use it. Would you be open to send a PR doing this work?

@anupamdalmia10
Copy link
Contributor Author

Sure, will raise a PR soon!

@josephwoodward
Copy link
Contributor

@anupamdalmia10 I'm happy to take this one off your hands if you find you don't have the time?

@anupamdalmia10
Copy link
Contributor Author

@josephwoodward, sure
I'll share my findings though, at the point where this Validate gets called, we do not have information about in which pipeline will be using this exporter.
So the only validation I could think of adding in this validation function is to check if everything(endpoint, traces_endpoint, metrics_endpoint, logs_endpoint) is blank. Not sure if we want to add this here. As this will also be evaluated when pipelines are being built.

@jpkrohling , comments?

@jpkrohling
Copy link
Member

We can still validate that there is at least one endpoint being specified.

@anupamdalmia10
Copy link
Contributor Author

Raised PR for this: #4860

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants