-
Notifications
You must be signed in to change notification settings - Fork 2.8k
[receiver/tlscheck] Move Target Validation to scrape & Implement Scrape Errors #40341
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
[receiver/tlscheck] Move Target Validation to scrape & Implement Scrape Errors #40341
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The implementation looks reasonable to me, though I would suggest documenting the change in error status and making sure all users can find the details of what they need to monitor.
if ct.Endpoint != "" && ct.FilePath != "" { | ||
return errors.New("cannot specify both endpoint and file_path") | ||
} | ||
if ct.Endpoint == "" && ct.FilePath == "" { | ||
return errors.New("must specify either endpoint or file_path") | ||
} | ||
|
||
// Validate endpoint if specified | ||
if ct.Endpoint != "" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do these changes mean the collector can now start in an invalid state? We normally like to catch that at startup.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We still do this validation, but we do it in the scrape function. The user can now include an invalid target, but it will be logged, the otelcol_scraper_errored_metric_points
will be incremented, and the collector will not crash. As currently implemented, the collector will fail to start if there is an invalid target in the TLS Check configuration, such as a nonexistent file certificate. I do not think this is desirable behavior for the user. The new behavior matches that of the File Stats Receiver, HTTP Check Receiver, and others.
Description
This PR moves target validation from
config.go
toscraper.go
. This causes a change in behavior. Previously, the receiver would crash if a target failed validation. With this change, the receiver will log an error message and increment theotelcol_scraper_errored_metric_points
metric.Testing
I added tests for the new validation function.