feat: upgrade observe-agent health check extension to health check v2#307
feat: upgrade observe-agent health check extension to health check v2#307obs-gh-ruoyuchen wants to merge 2 commits into
Conversation
![orca-security-us[bot]](https://avatars.githubusercontent.com/in/276250?s=60&v=4){kind=small}
There was a problem hiding this comment.
Orca Security Scan Summary
| Status | Check | Issues by priority | |
|---|---|---|---|
 Passed |
Infrastructure as Code |  0  0  0  0 |
View in Orca |
| Passed |
SAST | 0 0 0 0 |
View in Orca |
| Passed |
Secrets | 0 0 0 0 |
View in Orca |
| Passed |
Vulnerabilities | 0 0 0 0 |
View in Orca |
obs-gh-ruoyuchen
requested review from
obs-gh-abhishekrao and
obs-gh-mattcotter
obs-gh-mattcotter
left a comment
obs-gh-mattcotter
left a comment
There was a problem hiding this comment.
Looks great! A few comments on top of my two inline:
- Can you update the table in the README to reflect the new extension?
- Can you enable this test locally and make sure it passes: https://github.com/observeinc/observe-agent/blob/main/internal/commands/config/config_test.go#L102
| health_check: | ||
| endpoint: {{ .HealthCheck.Endpoint }} | ||
| path: {{ .HealthCheck.Path }} | ||
| healthcheckv2: |
There was a problem hiding this comment.
This looks great! Can you add a new bool config option in the HealthCheck config to toggle between the health_check extension and the healthcheckv2? See: https://github.com/observeinc/observe-agent/blob/main/internal/config/configschema.go#L52
I think we could call it use_v2 even though it wonn't math the OTel use_v2. If we default it to false, then we can experiment with the new extension without changing any user flows.
| github.com/open-telemetry/opentelemetry-collector-contrib/exporter/prometheusremotewriteexporter v0.144.0 | ||
| github.com/open-telemetry/opentelemetry-collector-contrib/exporter/syslogexporter v0.144.0 | ||
| github.com/open-telemetry/opentelemetry-collector-contrib/extension/cgroupruntimeextension v0.144.0 | ||
| github.com/open-telemetry/opentelemetry-collector-contrib/extension/healthcheckextension v0.144.0 |
There was a problem hiding this comment.
Let's leave the existing extension so we can toggle between the two. Even if we were doing a full replace with our bundled config, we would still want to support user overrides that rely on health_check existing.
2 participants
Description
Upgrade observe-agent health check extension to health check v2