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

Jump to bottom

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

Merged
merged 24 commits into from
Sep 6, 2023
+6,374 −181
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