Skip to content

Conversation

@jerm-dro
Copy link
Contributor

Summary

Fixes #3267

This PR adds another migration to finish migrating the other instance of the telemetry.Config in the runtime config.

Details

The original fix for the bug only migrated the telemetry_config field in the runtime config. However, it's also possible for telemetry.Config to exist in the runtimeConfig.middleware_configs[i].parameters.config as seen below:

{
  "middleware_configs": [
    {
      "type": "telemetry",
      "parameters": {
        "config": {
          "endpoint": "",
          "serviceName": "toolhive-mcp-proxy",
          "serviceVersion": "v0.6.13",
          "tracingEnabled": true,
          "metricsEnabled": true,
          "samplingRate": 1,
          "headers": {},
          "insecure": true,
          "enablePrometheusMetricsPath": true,
          "environmentVariables": [
            "USER",
            "HOST"
          ]
        },
        "server_name": "mermaid",
        "transport": "streamable-http"
      }
    }
  ]
}

How was this missed?
The parameters field is an untyped json blob.

Changes

  • refactor the telemetry migration so it runs against both telemetry_config and the middleware_configs.
  • introduce another migration to ensure the middleware_configs is migrated in the event users have already upgraded
  • add some tests to verify the migration of middleware_configs according to the structure above.

@jerm-dro jerm-dro requested a review from dmjb January 17, 2026 00:50
@github-actions github-actions bot added the size/M Medium PR: 300-599 lines changed label Jan 17, 2026
@github-actions github-actions bot added size/M Medium PR: 300-599 lines changed and removed size/M Medium PR: 300-599 lines changed labels Jan 17, 2026
@codecov
Copy link

codecov bot commented Jan 17, 2026

Codecov Report

❌ Patch coverage is 53.33333% with 42 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.75%. Comparing base (bc23d68) to head (808709a).
⚠️ Report is 38 commits behind head on main.

Files with missing lines Patch % Lines
pkg/migration/telemetry_config.go 59.15% 23 Missing and 6 partials ⚠️
pkg/migration/middleware_telemetry.go 0.00% 13 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3328      +/-   ##
==========================================
+ Coverage   64.62%   64.75%   +0.13%     
==========================================
  Files         369      373       +4     
  Lines       35953    36344     +391     
==========================================
+ Hits        23236    23536     +300     
- Misses      10890    10955      +65     
- Partials     1827     1853      +26     

☔ 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.

Copy link
Contributor

@jhrozek jhrozek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing the middleware telemetry config migration! I have a few questions about the implementation to make sure I understand the intended behavior.

Signed-off-by: Jeremy Drouillard <jeremy@stacklok.com>
@jerm-dro jerm-dro requested a review from jhrozek January 20, 2026 16:23
@github-actions github-actions bot added size/M Medium PR: 300-599 lines changed and removed size/M Medium PR: 300-599 lines changed labels Jan 20, 2026
@jerm-dro jerm-dro merged commit 6a18d9d into main Jan 21, 2026
56 of 58 checks passed
@jerm-dro jerm-dro deleted the jerm/2026-01-16-telem-migrate-2 branch January 21, 2026 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/M Medium PR: 300-599 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Noisy warning when loading workloads with telemetry config

3 participants