Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[extension/jaegerremotesampling]: Replace Thrift-gen with Proto-gen types for sampling strategies #18485

Merged
merged 24 commits into from
Sep 6, 2023
Merged

[extension/jaegerremotesampling]: Replace Thrift-gen with Proto-gen types for sampling strategies #18485

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
e24018d
[extension/jaegerremotesampling] move current state into deprecated pkg
frzifus Aug 25, 2023
9bd7d5c
[extension/jaegerremotesampling] remove mod files from deprecated pkg
frzifus Aug 25, 2023
72d7113
[extension/jaegerremotesampling] bump jaeger version from 1.41 to 1.48
frzifus Aug 25, 2023
0d315e7
[extension/jaegerremotesampling] register featuregate
frzifus Aug 25, 2023
5297c36
[pkg/translator/zipkin] bump to jaeger 1.48
frzifus Aug 25, 2023
4e94c5f
[receiver/jaegerreceiver] move current state into deprecated pkg
frzifus Aug 25, 2023
eaf7368
[receiver/jaegerreceiver] bump jaeger to 1.48
frzifus Aug 25, 2023
855a8de
[receiver/jaegerreceiver] register featuregate
frzifus Aug 25, 2023
fb46ce5
*: add changelog entry
frzifus Aug 25, 2023
07d1505
Update extension/jaegerremotesampling/factory.go
frzifus Aug 28, 2023
b990974
jaeger: add otel license header
frzifus Aug 30, 2023
c72a3e7
[extension/jaegerremotesampling] move old jaeger impl into internal
frzifus Aug 30, 2023
e8fe9c1
[receiver/jaegerreceiver] move old jaeger impl into internal
frzifus Aug 30, 2023
e660754
fix pkg paths
frzifus Aug 30, 2023
f80dfe5
*: make gotidy
frzifus Aug 30, 2023
497f7e2
fix
frzifus Aug 30, 2023
3982190
[extension/jaegerremotesampling] fix linting
frzifus Aug 31, 2023
b3045aa
[receiver/jaegerreceiver] fix linting
frzifus Aug 31, 2023
6468cce
Merge branch 'main' into jaegerremotesampling_replace_thrift-gen_with…
frzifus Aug 31, 2023
8e96c1c
Merge branch 'main' into jaegerremotesampling_replace_thrift-gen_with…
frzifus Aug 31, 2023
a4f2962
fix
frzifus Aug 31, 2023
f99ee72
Merge branch 'main' into jaegerremotesampling_replace_thrift-gen_with…
frzifus Aug 31, 2023
c542a2b
Merge branch 'main' into jaegerremotesampling_replace_thrift-gen_with…
frzifus Sep 4, 2023
25bd3b9
Merge branch 'main' into jaegerremotesampling_replace_thrift-gen_with…
frzifus Sep 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions .chloggen/jaegerremotesampling_replace_thrift-gen_with_proto-gen.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: enhancement

# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
component: jaegerreceiver,jaegerremotesamplingextension
Copy link
Member Author

Choose a reason for hiding this comment

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

After adding a featuregate to the jaegerremotesamplingextension the module worked fine, but it was not possible to compile the contrib client. Since otelcontribcol used 1.48.

Log 
cd ./cmd/otelcontribcol && GO111MODULE=on CGO_ENABLED=0 go build -trimpath -o ../../bin/otelcontribcol_linux_amd64 \
	-tags "" .
# github.com/open-telemetry/opentelemetry-collector-contrib/receiver/jaegerreceiver
../../receiver/jaegerreceiver/trace_receiver.go:174:43: cannot use (*notImplementedConfigManager)(nil) (value of type *notImplementedConfigManager) as configmanager.ClientConfigManager value in variable declaration: *notImplementedConfigManager does not implement configmanager.ClientConfigManager (wrong type for method GetSamplingStrategy)
		have GetSamplingStrategy(context.Context, string) (*"github.com/jaegertracing/jaeger/thrift-gen/sampling".SamplingStrategyResponse, error)
		want GetSamplingStrategy(context.Context, string) (*api_v2.SamplingStrategyResponse, error)
../../receiver/jaegerreceiver/trace_receiver.go:281:74: cannot use &notImplementedConfigManager{} (value of type *notImplementedConfigManager) as configmanager.ClientConfigManager value in argument to httpserver.NewHTTPServer: *notImplementedConfigManager does not implement configmanager.ClientConfigManager (wrong type for method GetSamplingStrategy)
		have GetSamplingStrategy(context.Context, string) (*"github.com/jaegertracing/jaeger/thrift-gen/sampling".SamplingStrategyResponse, error)
		want GetSamplingStrategy(context.Context, string) (*api_v2.SamplingStrategyResponse, error)
# github.com/open-telemetry/opentelemetry-collector-contrib/pkg/translator/zipkin/zipkinv1
../../pkg/translator/zipkin/zipkinv1/thrift.go:27:47: not enough arguments in call to jaegerzipkin.DeserializeThrift
	have ([]byte)
	want (context.Context, []byte)
make: *** [Makefile:287: otelcontribcol] Fehler 1

Therefore, I have increased the version there the same way and provided another feature gate. They work independently of each other.

Copy link
Member Author

Choose a reason for hiding this comment

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

cc @jpkrohling @zcross @MovieStoreGuy

Copy link
Member

Choose a reason for hiding this comment

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

As a temporary solution, it's fine.


# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Add featuregates to replace Thrift-gen with Proto-gen types for sampling strategies

# One or more tracking issues related to the change
issues: [18401]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext: |
Available featuregates are:
- extension.jaegerremotesampling.replaceThriftWithProto
- receiver.jaegerreceiver.replaceThriftWithProto
13 changes: 6 additions & 7 deletions cmd/configschema/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ require (
github.com/AzureAD/microsoft-authentication-library-for-go v1.0.0 // indirect
github.com/DataDog/opentelemetry-mapping-go/pkg/otlp/attributes v0.8.0 // indirect
github.com/DataDog/opentelemetry-mapping-go/pkg/otlp/metrics v0.8.0 // indirect
github.com/gocql/gocql v1.3.1 // indirect
github.com/gocql/gocql v1.3.2 // indirect
github.com/hailocab/go-hostpool v0.0.0-20160125115350-e80d13ce29ed // indirect
github.com/open-telemetry/opentelemetry-collector-contrib/exporter/alibabacloudlogserviceexporter v0.84.0 // indirect
github.com/open-telemetry/opentelemetry-collector-contrib/exporter/awscloudwatchlogsexporter v0.84.0 // indirect
Expand Down Expand Up @@ -430,7 +430,7 @@ require (
github.com/jackc/pgservicefile v0.0.0-20221227161230-091c0ba34f0a // indirect
github.com/jackc/pgtype v1.14.0 // indirect
github.com/jackc/pgx/v4 v4.18.1 // indirect
github.com/jaegertracing/jaeger v1.41.0 // indirect
github.com/jaegertracing/jaeger v1.48.0 // indirect
github.com/jcmturner/aescts/v2 v2.0.0 // indirect
github.com/jcmturner/dnsutils/v2 v2.0.0 // indirect
github.com/jcmturner/gofork v1.7.6 // indirect
Expand Down Expand Up @@ -537,8 +537,7 @@ require (
github.com/ovh/go-ovh v1.4.1 // indirect
github.com/patrickmn/go-cache v2.1.0+incompatible // indirect
github.com/paulmach/orb v0.10.0 // indirect
github.com/pelletier/go-toml v1.9.5 // indirect
github.com/pelletier/go-toml/v2 v2.0.5 // indirect
github.com/pelletier/go-toml/v2 v2.0.8 // indirect
github.com/philhofer/fwd v1.1.2 // indirect
github.com/pierrec/lz4 v2.6.1+incompatible // indirect
github.com/pierrec/lz4/v4 v4.1.18 // indirect
Expand Down Expand Up @@ -574,14 +573,14 @@ require (
github.com/snowflakedb/gosnowflake v1.6.24 // indirect
github.com/soheilhy/cmux v0.1.5 // indirect
github.com/spaolacci/murmur3 v1.1.0 // indirect
github.com/spf13/afero v1.9.3 // indirect
github.com/spf13/afero v1.9.5 // indirect
github.com/spf13/cast v1.5.1 // indirect
github.com/spf13/cobra v1.7.0 // indirect
github.com/spf13/jwalterweatherman v1.1.0 // indirect
github.com/spf13/pflag v1.0.5 // indirect
github.com/spf13/viper v1.14.0 // indirect
github.com/spf13/viper v1.16.0 // indirect
github.com/stretchr/objx v0.5.0 // indirect
github.com/subosito/gotenv v1.4.1 // indirect
github.com/subosito/gotenv v1.4.2 // indirect
github.com/syndtr/gocapability v0.0.0-20200815063812-42c35b437635 // indirect
github.com/tencentcloud/tencentcloud-sdk-go/tencentcloud/common v1.0.741 // indirect
github.com/tg123/go-htpasswd v1.2.1 // indirect
Expand Down
23 changes: 13 additions & 10 deletions cmd/configschema/go.sum
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 6 additions & 7 deletions cmd/otelcontribcol/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,7 @@ require (
github.com/go-zookeeper/zk v1.0.3 // indirect
github.com/gobwas/glob v0.2.3 // indirect
github.com/goccy/go-json v0.10.2 // indirect
github.com/gocql/gocql v1.3.1 // indirect
github.com/gocql/gocql v1.3.2 // indirect
github.com/godbus/dbus v0.0.0-20190726142602-4481cbc300e2 // indirect
github.com/godbus/dbus/v5 v5.1.0 // indirect
github.com/gofrs/uuid v4.2.0+incompatible // indirect
Expand Down Expand Up @@ -454,7 +454,7 @@ require (
github.com/jackc/pgservicefile v0.0.0-20221227161230-091c0ba34f0a // indirect
github.com/jackc/pgtype v1.14.0 // indirect
github.com/jackc/pgx/v4 v4.18.1 // indirect
github.com/jaegertracing/jaeger v1.41.0 // indirect
github.com/jaegertracing/jaeger v1.48.0 // indirect
github.com/jcmturner/aescts/v2 v2.0.0 // indirect
github.com/jcmturner/dnsutils/v2 v2.0.0 // indirect
github.com/jcmturner/gofork v1.7.6 // indirect
Expand Down Expand Up @@ -556,8 +556,7 @@ require (
github.com/ovh/go-ovh v1.4.1 // indirect
github.com/patrickmn/go-cache v2.1.0+incompatible // indirect
github.com/paulmach/orb v0.10.0 // indirect
github.com/pelletier/go-toml v1.9.5 // indirect
github.com/pelletier/go-toml/v2 v2.0.5 // indirect
github.com/pelletier/go-toml/v2 v2.0.8 // indirect
github.com/philhofer/fwd v1.1.2 // indirect
github.com/pierrec/lz4 v2.6.1+incompatible // indirect
github.com/pierrec/lz4/v4 v4.1.18 // indirect
Expand Down Expand Up @@ -595,14 +594,14 @@ require (
github.com/snowflakedb/gosnowflake v1.6.24 // indirect
github.com/soheilhy/cmux v0.1.5 // indirect
github.com/spaolacci/murmur3 v1.1.0 // indirect
github.com/spf13/afero v1.9.2 // indirect
github.com/spf13/afero v1.9.5 // indirect
github.com/spf13/cast v1.5.1 // indirect
github.com/spf13/cobra v1.7.0 // indirect
github.com/spf13/jwalterweatherman v1.1.0 // indirect
github.com/spf13/pflag v1.0.5 // indirect
github.com/spf13/viper v1.14.0 // indirect
github.com/spf13/viper v1.16.0 // indirect
github.com/stretchr/objx v0.5.0 // indirect
github.com/subosito/gotenv v1.4.1 // indirect
github.com/subosito/gotenv v1.4.2 // indirect
github.com/syndtr/gocapability v0.0.0-20200815063812-42c35b437635 // indirect
github.com/tencentcloud/tencentcloud-sdk-go/tencentcloud/common v1.0.741 // indirect
github.com/tg123/go-htpasswd v1.2.1 // indirect
Expand Down
Loading