Moving prometheus check to validate step #2057
Conversation
confgenerator/config.go
Outdated
| } | ||
| receiverPipelines, err := receiver.Pipelines(ctx) | ||
| if err != nil { | ||
| continue |
There was a problem hiding this comment.
Why continue here instead of return err?
confgenerator/config.go
Outdated
| for _, receiverPipeline := range receiverPipelines { | ||
| // Check the Ops Agent receiver type. | ||
| if receiverPipeline.ExporterTypes[subagent] == otel.GMP { | ||
| fmt.Println("processorIDs", processorIDs) |
There was a problem hiding this comment.
Remove the println here?
confgenerator/config.go
Outdated
| return nil | ||
| } | ||
|
|
||
| func validatePrometheusReceiver(receivers metricsReceiverMap, receiverIDs, processorIDs []string, subagent string, ctx context.Context) error { |
There was a problem hiding this comment.
[nit] naming is hard but we should either rename or add a comment about why we do this validation. Or do both.
maybe something like validateNoCustomGMPProcessors or sth idk. What we're validating here is that if we are sending to GMP, then Ops Agent users can't configure any processors in this pipeline
confgenerator/config.go
Outdated
| for _, ID := range receiverIDs { | ||
| receiver, ok := receivers[ID] | ||
| if !ok { | ||
| continue // Some receivers might not be used. |
There was a problem hiding this comment.
Actually reading this again - this looks incorrect. If !ok, then it looks like a receiver is used in the pipeline but there is no such receiver in the map?
Does another validator catch this error already or must we also return an error here?
There was a problem hiding this comment.
Looks like validateComponentKeys will catch this for us. I would call that out and also return an error here but this error would never actually happen. Maybe we can panic too
Description
This moves the check for processors in prometheus pipeline to the validate stage.
Related issue
b/435217702
How has this been tested?
Checklist: