Skip to content

Conversation

@disq
Copy link
Member

@disq disq commented Dec 18, 2024

resource.Merge returns an error only when schemas don't match.

@disq disq requested review from a team and ronsh12 December 18, 2024 21:59
@github-actions github-actions bot added the fix label Dec 18, 2024
@disq disq requested review from erezrokah and removed request for ronsh12 December 18, 2024 22:00
if schemaURL == "" {
schemaURL = semconv.SchemaURL
}
r, err := resource.Merge(
Copy link
Member Author

@disq disq Dec 18, 2024

Choose a reason for hiding this comment

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

This method always returns r (even with err, which the single value can be ErrSchemaURLConflict) so we could also do if r != nil { return r } panic(err) but in that case the resource's schema URL could be empty.

@erezrokah
Copy link
Member

Is there a reproduction for the panic? Based on the panic stack:

panic: conflicting Schema URL: https://opentelemetry.io/schemas/1.26.0 and https://opentelemetry.io/schemas/1.24.0
goroutine 1 [running]:
github.com/cloudquery/plugin-sdk/v4/serve.newResource(0xc0006e92c0)
	/go/pkg/mod/github.com/cloudquery/plugin-sdk/v4@v4.38.2/serve/opentelemetry.go:21 +0x2e5
github.com/cloudquery/plugin-sdk/v4/serve.(*PluginServe).newCmdPluginServe.func1(0xc00063c908, {0xc0000fd600?, 0x4?, 0x14d0c66?})
	/go/pkg/mod/github.com/cloudquery/plugin-sdk/v4@v4.38.2/serve/plugin.go:153 +0x34e
github.com/spf13/cobra.(*Command).execute(0xc00063c908, {0xc0000fd550, 0xb, 0xb})
	/go/pkg/mod/github.com/spf13/cobra@v1.8.0/command.go:983 +0xaaa
github.com/spf13/cobra.(*Command).ExecuteC(0xc00063c608)
	/go/pkg/mod/github.com/spf13/cobra@v1.8.0/command.go:1115 +0x3ff
github.com/spf13/cobra.(*Command).Execute(...)
	/go/pkg/mod/github.com/spf13/cobra@v1.8.0/command.go:1039
github.com/spf13/cobra.(*Command).ExecuteContext(...)
	/go/pkg/mod/github.com/spf13/cobra@v1.8.0/command.go:1032
github.com/cloudquery/plugin-sdk/v4/serve.(*PluginServe).Serve(0xc0005d8540, {0x1726860, 0x220f220})
	/go/pkg/mod/github.com/cloudquery/plugin-sdk/v4@v4.38.2/serve/plugin.go:114 +0xee
main.main()
	/go/main.go:13 +0xaa

The issue is from an old SDK version (introduced in https://github.com/cloudquery/plugin-sdk/pull/1634/files) and fixed via https://github.com/cloudquery/plugin-sdk/pull/1605/files and https://github.com/cloudquery/plugin-sdk/pull/1607/files

@erezrokah
Copy link
Member

OK something doesn't add up as v4.38.2 was released after #1605

Copy link
Member

@erezrokah erezrokah left a comment

Choose a reason for hiding this comment

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

OK let's go with this fix though not sure how the mismatch can happen, probably related to the logic in https://github.com/open-telemetry/opentelemetry-go/blob/82c57cb8dd23ab537e60fbd1625bbb88b163f222/sdk/resource/resource.go#L230

@kodiakhq kodiakhq bot merged commit b616279 into main Dec 19, 2024
8 checks passed
@kodiakhq kodiakhq bot deleted the fix/otel-schema-url-panic branch December 19, 2024 09:57
kodiakhq bot pushed a commit that referenced this pull request Dec 19, 2024
kodiakhq bot pushed a commit that referenced this pull request Dec 19, 2024
🤖 I have created a release *beep* *boop*
---


## [4.72.1](v4.72.0...v4.72.1) (2024-12-19)


### Bug Fixes

* **deps:** Update aws-sdk-go-v2 monorepo ([#2007](#2007)) ([7f3818d](7f3818d))
* **deps:** Update module github.com/cloudquery/cloudquery-api-go to v1.13.5 ([#2015](#2015)) ([9b6e9f2](9b6e9f2))
* **deps:** Update module github.com/cloudquery/plugin-pb-go to v1.26.1 ([#2010](#2010)) ([b12dc10](b12dc10))
* **deps:** Update module golang.org/x/net to v0.33.0 [SECURITY] ([#2014](#2014)) ([7360bd2](7360bd2))
* **deps:** Update module google.golang.org/grpc to v1.69.0 ([#2008](#2008)) ([aae018f](aae018f))
* OpenTelemetry schema URL panic ([#2012](#2012)) ([b616279](b616279))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants