search

open-telemetry / opentelemetry-collector-contrib

Contrib repository for the OpenTelemetry Collector
https://opentelemetry.io
Apache License 2.0
2.92k stars 2.28k forks source link

[CI] Check consistent version usage of core collector dependencies #8546

Closed mx-psi closed 2 years ago

mx-psi commented 2 years ago

A common issue (e.g. most recently found on the v0.47.0 release cycle) is for individual components that are not yet added to the main build to require a Collector version lower than the one on the main go.mod file. This makes it more difficult to bump the Collector version since, when we do that, it uncovers hidden issues.

We can build a small Go tool using the modfile parser to walk over every go.mod file on the repository and check that the required version for go.opentelemetry.io/collector/* dependencies is the same as the one required in the main go.mod file.

Aneurysm9 commented 2 years ago

The multimod sync command should be able to handle this. We use it in the go-contrib repo for this same purpose.

hickeyma commented 2 years ago

Does Dependabot pickup the version change? Or am I missing something?

mx-psi commented 2 years ago

Does Dependabot pickup the version change? Or am I missing something?

This would ideally happen at the PR level: if someone develops a new component, its go.mod file won't be considered by Dependabot since it won't be on the main branch. And even in the main branch, Dependabot does not immediately pick up new versions nor does it work with pseudoversions, so it wouldn't work most of the time.

hickeyma commented 2 years ago

Thanks for feedback @mx-psi.

I can take a look at multimod sync command as used in go-contrib if that's ok?

mx-psi commented 2 years ago

I am not familiar with the multimod sync command but I think it's worth giving it a try, and probably @Aneurysm9 can help with any questions

Aneurysm9 commented 2 years ago

Thinking about this again, isn't this covered by the make update-otel target? The default version for that is main and would cover the cases where we need to ensure the latest core is used between releases. multimod sync would be useful at release time as it will sync to a given tag, not a pseudoversion.

Can we confirm that all components have a symlink to Makefile.common and get updated appropriately with make update-otel?

hickeyma commented 2 years ago

Can we confirm that all components have a symlink to Makefile.common and get updated appropriately with make update-otel?

@Aneurysm9 I'll take a look.

hickeyma commented 2 years ago

Can we confirm that all components have a symlink to Makefile.common and get updated appropriately with make update-otel?

Yes, they seem to get updated appropriately. Here is example of output from running the update-otel target:

[...]
/Library/Developer/CommandLineTools/usr/bin/make -C ./exporter/loadbalancingexporter updatedep
/Users/mhickey/go/src/github.com/open-telemetry/opentelemetry-collector-contrib/internal/buildscripts/update-dep
go get: upgraded go.opentelemetry.io/collector v0.47.1-0.20220323200141-1b7618728835 => v0.47.1-0.20220324182827-cb868e20c8ff
go get: upgraded go.opentelemetry.io/collector/model v0.47.1-0.20220323200141-1b7618728835 => v0.47.1-0.20220324182827-cb868e20c8ff
rm -fr go.sum
go mod tidy -compat=1.17
[...]

Wrt symlink to Makefile.common, I checked as follows:

$ for i in $(find . -name Makefile); do                     
   wc -l $i
done

       1 ./cmd/mdatagen/Makefile
       1 ./cmd/configschema/Makefile
       0 ./tracegen/Makefile
       1 ./extension/sigv4authextension/Makefile
       0 ./extension/fluentbitextension/Makefile
       1 ./extension/oidcauthextension/Makefile
       1 ./extension/asapauthextension/Makefile
       1 ./extension/bearertokenauthextension/Makefile
       0 ./extension/oauth2clientauthextension/Makefile
       0 ./extension/httpforwarder/Makefile
       1 ./extension/pprofextension/Makefile
       1 ./extension/observer/Makefile
       1 ./extension/observer/k8sobserver/Makefile
       1 ./extension/observer/dockerobserver/Makefile
       1 ./extension/observer/ecsobserver/Makefile
       1 ./extension/observer/hostobserver/Makefile
       1 ./extension/observer/ecstaskobserver/Makefile
       0 ./extension/awsproxy/Makefile
       1 ./extension/storage/Makefile
       1 ./extension/storage/storagetest/Makefile
       2 ./extension/healthcheckextension/Makefile
       1 ./extension/basicauthextension/Makefile
       0 ./extension/jaegerremotesampling/Makefile
      37 ./testbed/Makefile
       1 ./testbed/mockdatareceivers/mockawsxrayreceiver/Makefile
     309 ./Makefile
       0 ./processor/cumulativetodeltaprocessor/Makefile
       1 ./processor/spanprocessor/Makefile
       1 ./processor/metricstransformprocessor/Makefile
       0 ./processor/tailsamplingprocessor/Makefile
       1 ./processor/probabilisticsamplerprocessor/Makefile
       0 ./processor/deltatorateprocessor/Makefile
       1 ./processor/groupbyattrsprocessor/Makefile
       1 ./processor/metricsgenerationprocessor/Makefile
       1 ./processor/attributesprocessor/Makefile
       1 ./processor/filterprocessor/Makefile
       0 ./processor/groupbytraceprocessor/Makefile
       1 ./processor/resourcedetectionprocessor/Makefile
       0 ./processor/routingprocessor/Makefile
       1 ./processor/transformprocessor/Makefile
       0 ./processor/k8sattributesprocessor/Makefile
       1 ./processor/resourceprocessor/Makefile
       1 ./processor/redactionprocessor/Makefile
       0 ./processor/spanmetricsprocessor/Makefile
       1 ./internal/tools/Makefile
       1 ./internal/docker/Makefile
       1 ./internal/stanza/Makefile
       1 ./internal/sharedcomponent/Makefile
       1 ./internal/containertest/Makefile
       1 ./internal/coreinternal/Makefile
       1 ./internal/common/Makefile
       1 ./internal/splunk/Makefile
       1 ./internal/k8sconfig/Makefile
       1 ./internal/scrapertest/Makefile
       1 ./internal/kubelet/Makefile
       1 ./internal/aws/metrics/Makefile
       1 ./internal/aws/cwlogs/Makefile
       1 ./internal/aws/containerinsight/Makefile
       1 ./internal/aws/proxy/Makefile
       1 ./internal/aws/awsutil/Makefile
       1 ./internal/aws/ecsutil/Makefile
       1 ./internal/aws/k8s/Makefile
       1 ./internal/aws/xray/testdata/sampleserver/Makefile
       1 ./internal/aws/xray/testdata/sampleapp/Makefile
       1 ./internal/aws/xray/Makefile
       1 ./examples/demo/server/Makefile
       1 ./examples/demo/client/Makefile
       1 ./receiver/kafkareceiver/Makefile
       1 ./receiver/windowsperfcountersreceiver/Makefile
       1 ./receiver/elasticsearchreceiver/Makefile
       1 ./receiver/hostmetricsreceiver/Makefile
       1 ./receiver/dotnetdiagnosticsreceiver/Makefile
       1 ./receiver/rabbitmqreceiver/Makefile
       0 ./receiver/awsxrayreceiver/Makefile
       0 ./receiver/googlecloudspannerreceiver/Makefile
       1 ./receiver/couchdbreceiver/Makefile
       0 ./receiver/fluentforwardreceiver/Makefile
       1 ./receiver/prometheusexecreceiver/Makefile
       1 ./receiver/mongodbreceiver/Makefile
       1 ./receiver/nginxreceiver/Makefile
       1 ./receiver/postgresqlreceiver/Makefile
       1 ./receiver/filelogreceiver/Makefile
       0 ./receiver/statsdreceiver/Makefile
       0 ./receiver/influxdbreceiver/Makefile
       1 ./receiver/k8seventsreceiver/Makefile
       1 ./receiver/journaldreceiver/Makefile
       2 ./receiver/kubeletstatsreceiver/Makefile
       1 ./receiver/couchbasereceiver/Makefile
       0 ./receiver/awsecscontainermetricsreceiver/Makefile
       1 ./receiver/tcplogreceiver/Makefile
       0 ./receiver/redisreceiver/Makefile
       1 ./receiver/podmanreceiver/Makefile
       0 ./receiver/splunkhecreceiver/Makefile
       1 ./receiver/jaegerreceiver/Makefile
       0 ./receiver/signalfxreceiver/Makefile
       0 ./receiver/zookeeperreceiver/Makefile
       1 ./receiver/opencensusreceiver/Makefile
       1 ./receiver/kafkametricsreceiver/Makefile
       1 ./receiver/jmxreceiver/Makefile
       1 ./receiver/jmxreceiver/internal/subprocess/Makefile
       0 ./receiver/memcachedreceiver/Makefile
       1 ./receiver/mysqlreceiver/Makefile
       0 ./receiver/carbonreceiver/Makefile
       1 ./receiver/awsfirehosereceiver/Makefile
       1 ./receiver/zipkinreceiver/Makefile
       0 ./receiver/simpleprometheusreceiver/Makefile
       0 ./receiver/simpleprometheusreceiver/examples/federation/prom-counter/Makefile
       1 ./receiver/udplogreceiver/Makefile
       1 ./receiver/mongodbatlasreceiver/Makefile
       0 ./receiver/wavefrontreceiver/Makefile
       1 ./receiver/syslogreceiver/Makefile
       0 ./receiver/collectdreceiver/Makefile
       0 ./receiver/cloudfoundryreceiver/Makefile
       1 ./receiver/dockerstatsreceiver/Makefile
       1 ./receiver/receivercreator/Makefile
       0 ./receiver/googlecloudpubsubreceiver/Makefile
       1 ./receiver/prometheusreceiver/Makefile
       1 ./receiver/skywalkingreceiver/Makefile
       1 ./receiver/apachereceiver/Makefile
       0 ./receiver/awscontainerinsightreceiver/Makefile
       0 ./receiver/k8sclusterreceiver/Makefile
       1 ./receiver/sapmreceiver/Makefile
       0 ./exporter/loadbalancingexporter/Makefile
       1 ./exporter/fileexporter/Makefile
       1 ./exporter/jaegerexporter/Makefile
       1 ./exporter/signalfxexporter/Makefile
       1 ./exporter/splunkhecexporter/Makefile
       1 ./exporter/awscloudwatchlogsexporter/Makefile
       1 ./exporter/awsprometheusremotewriteexporter/Makefile
       0 ./exporter/awskinesisexporter/Makefile
       1 ./exporter/influxdbexporter/Makefile
       0 ./exporter/logzioexporter/Makefile
       1 ./exporter/dynatraceexporter/Makefile
       1 ./exporter/newrelicexporter/Makefile
       1 ./exporter/awsxrayexporter/Makefile
       0 ./exporter/elasticsearchexporter/Makefile
       0 ./exporter/awsemfexporter/Makefile
       1 ./exporter/kafkaexporter/Makefile
       0 ./exporter/stackdriverexporter/Makefile
       1 ./exporter/humioexporter/Makefile
       1 ./exporter/jaegerthrifthttpexporter/Makefile
       1 ./exporter/skywalkingexporter/Makefile
       0 ./exporter/sapmexporter/Makefile
       1 ./exporter/coralogixexporter/Makefile
       1 ./exporter/tanzuobservabilityexporter/Makefile
       1 ./exporter/sentryexporter/Makefile
       1 ./exporter/parquetexporter/Makefile
       0 ./exporter/googlecloudpubsubexporter/Makefile
       1 ./exporter/prometheusexporter/Makefile
       0 ./exporter/azuremonitorexporter/Makefile
       1 ./exporter/f5cloudexporter/Makefile
       0 ./exporter/observiqexporter/Makefile
       1 ./exporter/prometheusremotewriteexporter/Makefile
       1 ./exporter/lokiexporter/Makefile
       1 ./exporter/zipkinexporter/Makefile
       1 ./exporter/carbonexporter/Makefile
       1 ./exporter/tencentcloudlogserviceexporter/Makefile
       1 ./exporter/sumologicexporter/Makefile
       0 ./exporter/googlecloudexporter/Makefile
       8 ./exporter/clickhouseexporter/Makefile
       0 ./exporter/elasticexporter/Makefile
       1 ./exporter/datadogexporter/Makefile
       1 ./exporter/honeycombexporter/Makefile
       1 ./exporter/opencensusexporter/Makefile
       1 ./exporter/alibabacloudlogserviceexporter/Makefile
       1 ./pkg/translator/opencensus/Makefile
       1 ./pkg/translator/prometheusremotewrite/Makefile
       1 ./pkg/translator/signalfx/Makefile
       1 ./pkg/translator/jaeger/Makefile
       1 ./pkg/translator/zipkin/Makefile
       0 ./pkg/batchpersignal/Makefile
       0 ./pkg/batchperresourceattr/Makefile
       1 ./pkg/resourcetotelemetry/Makefile
       1 ./pkg/experimentalmetricmetadata/Makefile

I have ascertained that a line count of 0, 1, or 2 should be just a link to Makefile.common. In other words: line without linefeed, one line or one line with an additional empty line. The following Makefile differed: ./testbed/Makefile, ./Makefile and ./exporter/clickhouseexporter/Makefile.

hickeyma commented 2 years ago

I have opened #8837 as the update-otel target is failing because of the go-chi dependency issue. I pushed a PR to fix it for the time being until the dependency issue is resolved.

hickeyma commented 2 years ago

@Aneurysm9 Does the response in https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/8546#issuecomment-1078432565 cover the points you raised in https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/8546#issuecomment-1076758646?

Aneurysm9 commented 2 years ago

So, is there actually an issue here, then? If make update-otel is hitting all of the component modules then we should be good, right?

hickeyma commented 2 years ago

So, is there actually an issue here, then? If make update-otel is hitting all of the component modules then we should be good, right?

Seems to me. Does this cover the issue for you @mx-psi ?

mx-psi commented 2 years ago

I made a bad job of explaining this, sorry about that. My intention is to avoid the following situation:

  1. Someone creates a PR adding a new examplemodule module, targeting Collector core vN module
  2. Weeks pass, it gets reviewed and merged just before the vN+3 contrib release, still targeting vN
  3. The release manager for vN+3 has then to manually update the examplemodule module code to be compatible with vN+3

I want to avoid (3) and, instead, make the contributor adding examplemodule do that job before merging the PR, by comparing the Collector core version at the top-level go.mod file with the Collector core version at the examplemodule go.mod file.

Does that make more sense?

hickeyma commented 2 years ago

@mx-psi Thanks for the description. I will dig into this and get back to you shortly.