Skip to content

Conversation

@jade-guiton-dd
Copy link
Contributor

@jade-guiton-dd jade-guiton-dd commented Nov 27, 2025

Description

Follow up to open-telemetry/opentelemetry-collector#14190, which introduced an API breaking change: instead of passing a host component.Host to configgrpc and confighttp's ToClient / ToServer methods, components should instead pass the result of host.GetComponents(). Tests that don't make use of authentication or middleware extensions, as well as custom component hosts which don't support extensions (such as the OpAMP supervisor), can simply pass nil instead of using a no-op host.

Replacing host by host.GetComponents() is fairly straightforward, as long as you don't pass in a nil host, which unfortunately a lot of tests do, so I had to fix that as well.

This will be merged into the update-otel PR at #44581.

@jade-guiton-dd
Copy link
Contributor Author

jade-guiton-dd commented Nov 27, 2025

Pinging Sumologic extension code owners: @rnishtala-sumo @chan-tim-sumo @amdprophet

The TestUpdateMetadataRequestPayload test in extension/sumologicextension/extension_test.go calls SumologicExtension.getHTTPClient without calling SumologicExtension.Start first, which leads to confighttp.ClientConfig.ToClient being called with a nil host. This used to work as long as no authentication/middleware was configured, but it poses a problem now that components are expected to call host.GetExtensions() upfront, before calling ToClient; the most straightforward fix of the extension code leads to a segfault in the test.

I tried adding a call to Start in between, but I think the heartbeat loop started in the process causes the test to fail very verbosely. So I just manually set SumologicExtension.host to a no-op host instead. This is not a very elegant fix, so I will leave it to y'all to figure out what you want to do long term. Presumably, this would involve either:

  • considering a nil SumologicExtension.host to be a valid state, and putting a condition around the call to GetExtensions(), or
  • modifying the test to start the component with a valid host, but in a way which doesn't fail the test.

@jade-guiton-dd
Copy link
Contributor Author

After the last commit, the scoped tests failed twice, but because of flaky tests that seem unrelated to the changes, so I'm marking this ready for review.

I have no idea why changing nil to componenttest.NewNopHost() caused a failure in processor/metricsgenerationprocessor/processor_test.go, considering the component doesn't seem to define a Start handler or use the host at all... It was an unnecessary change for the purposes of this PR so I reverted it, but maybe something to investigate.

Copy link
Member

@mx-psi mx-psi left a comment

Choose a reason for hiding this comment

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

I am going to merge before scoped tests pass since this is a PR against another PR

@mx-psi mx-psi merged commit 3e38397 into open-telemetry:otelbot/update-otel-1764244003 Nov 28, 2025
179 of 191 checks passed
@github-actions github-actions bot added this to the next release milestone Nov 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cmd/opampsupervisor connector/grafanacloud exporter/alertmanager exporter/azuremonitor exporter/bmchelix exporter/coralogix exporter/datadog Datadog components exporter/doris exporter/elasticsearch exporter/faro exporter/honeycombmarker exporter/influxdb exporter/logicmonitor exporter/logzio exporter/mezmo exporter/opensearch exporter/otelarrow exporter/prometheus exporter/prometheusremotewrite exporter/sematext exporter/signalfx exporter/splunkhec exporter/stef exporter/sumologic exporter/tinybird exporter/zipkin extension/datadog extension/encoding/otlpencoding extension/healthcheck Health Check Extension extension/httpforwarder extension/jaegerremotesampling extension/remotetap extension/solarwindsapmsettings extension/sumologic internal/aws internal/healthcheck pkg/stanza processor/cumulativetodelta Cumulative To Delta processor processor/deltatocumulative processor/deltatorate Delta To Rate processor processor/filter Filter processor processor/groupbytrace Group By Trace processor processor/isolationforest processor/logstransform Logs Transform processor processor/remotetap processor/resourcedetection Resource detection processor processor/schema Schema processor receiver/apache receiver/apachespark receiver/awscontainerinsight receiver/awsfirehose receiver/bigip receiver/cloudfoundry receiver/collectd receiver/couchdb receiver/datadog receiver/elasticsearch receiver/envoyals receiver/expvar receiver/faro receiver/flinkmetrics receiver/fluentforward receiver/github receiver/gitlab receiver/googlecloudpubsubpush receiver/haproxy receiver/httpcheck HTTP Check receiver receiver/influxdb receiver/jaeger receiver/kafkametrics receiver/kubeletstats receiver/libhoney receiver/loki receiver/nginx receiver/nsxt NSXT Receiver receiver/otelarrow

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants