Skip to content

Conversation

@danbarr
Copy link
Collaborator

@danbarr danbarr commented Sep 15, 2025

Fixes #1899

The insecure and enable-prometheus-metrics-path configs were not falling back to the global config file settings when the CLI flags aren't provided as overrides. This caused ToolHive to:

  • Attempt HTTPS connections even when thv config otel set-insecure true was configured
  • Not expose the Prometheus endoing even when thv config otel set-enable-prometheus-metrics-path true was configured

Changes:

  • Added fallback logic for insecure and enable-prometheus-metrics-path settings in getTelemetryFromFlags()
  • Enhanced tests to verify fallback behavior of the two new flags

NOTE: this issue also affects the metrics-enabled and tracing-enabled flags that were also added in #1785, but since these default to true, I had trouble solving for them in the same way, since unset and explicit false config settings are currently indistinguishable.

Signed-off-by: Dan Barr <6922515+danbarr@users.noreply.github.com>
@codecov
Copy link

codecov bot commented Sep 15, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 46.39%. Comparing base (f917be9) to head (e0dfbec).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1900      +/-   ##
==========================================
+ Coverage   45.77%   46.39%   +0.62%     
==========================================
  Files         203      206       +3     
  Lines       25979    26145     +166     
==========================================
+ Hits        11891    12131     +240     
+ Misses      13173    13094      -79     
- Partials      915      920       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

- Add fallback logic for OtelEnablePrometheusMetricsPath in getTelemetryFromFlags()
- Update function signature to handle the additional parameter
- Enhanced tests to verify prometheus metrics path fallback behavior

Same issue as the insecure setting - was being passed directly without
checking the global config fallback.
@danbarr danbarr changed the title Fix OTel insecure setting fallback to global config Fix OTel insecure and prometheus settings fallback to global config Sep 15, 2025
Signed-off-by: Dan Barr <6922515+danbarr@users.noreply.github.com>
Signed-off-by: Dan Barr <6922515+danbarr@users.noreply.github.com>
@danbarr danbarr merged commit 161d000 into main Sep 16, 2025
29 of 31 checks passed
@danbarr danbarr deleted the fix/otel-insecure-config branch September 16, 2025 00:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OTel insecure setting isn't working in global config

3 participants