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.

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

[Fix]: update all otel collector packages #6759

Merged
merged 6 commits into from
Feb 20, 2025

Conversation

AnmolxSingh
Copy link
Contributor

Which problem is this PR solving?

Description of the changes

  • Done minor code change of deprecated symbols after bot provided upgrade

How was this change tested?

Checklist

Signed-off-by: AnmolxSingh <[email protected]>
@AnmolxSingh AnmolxSingh requested a review from a team as a code owner February 19, 2025 19:35
@dosubot dosubot bot added the area/otel label Feb 19, 2025
@AnmolxSingh
Copy link
Contributor Author

@yurishkuro please review

Signed-off-by: AnmolxSingh <[email protected]>
Copy link

codecov bot commented Feb 19, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.06%. Comparing base (46a7096) to head (4aa4ee5).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6759      +/-   ##
==========================================
+ Coverage   96.02%   96.06%   +0.03%     
==========================================
  Files         364      364              
  Lines       20692    20692              
==========================================
+ Hits        19869    19877       +8     
+ Misses        628      622       -6     
+ Partials      195      193       -2     
Flag Coverage Δ
badger_v1 9.76% <ø> (ø)
badger_v2 1.82% <ø> (ø)
cassandra-4.x-v1-manual 14.81% <ø> (ø)
cassandra-4.x-v2-auto 1.81% <ø> (ø)
cassandra-4.x-v2-manual 1.81% <ø> (ø)
cassandra-5.x-v1-manual 14.81% <ø> (ø)
cassandra-5.x-v2-auto 1.81% <ø> (ø)
cassandra-5.x-v2-manual 1.81% <ø> (ø)
elasticsearch-6.x-v1 19.15% <ø> (ø)
elasticsearch-7.x-v1 19.23% <ø> (ø)
elasticsearch-8.x-v1 19.40% <ø> (ø)
elasticsearch-8.x-v2 1.82% <ø> (ø)
grpc_v1 10.81% <ø> (ø)
grpc_v2 7.80% <ø> (ø)
kafka-3.x-v1 10.13% <ø> (ø)
kafka-3.x-v2 1.82% <ø> (ø)
memory_v2 1.82% <ø> (ø)
opensearch-1.x-v1 19.28% <ø> (ø)
opensearch-2.x-v1 19.28% <ø> (ø)
opensearch-2.x-v2 1.82% <ø> (ø)
tailsampling-processor 0.48% <ø> (ø)
unittests 94.94% <100.00%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@yurishkuro yurishkuro added the changelog:dependencies Update to dependencies label Feb 19, 2025
@yurishkuro
Copy link
Member

unit tests are failing

@AnmolxSingh
Copy link
Contributor Author

AnmolxSingh commented Feb 20, 2025

While debugging my test failure is specific to fswatcher_test.go
Screenshot 2025-02-20 170734

@yurishkuro
Copy link
Member

It's not what failed in the ci

component type mismatch: component ID "" does not have type "remote_sampling"

@AnmolxSingh
Copy link
Contributor Author

Got it. Where could I find file where remote sampling is registered

@yurishkuro
Copy link
Member

Where could I find file where remote sampling is registered

not sure I follow. Just run the tests that are failing and see which lines. You can also run the tests in debugger

@AnmolxSingh
Copy link
Contributor Author

AnmolxSingh commented Feb 20, 2025

Screenshot 2025-02-20 235258
Screenshot 2025-02-20 235313

In cmd\jaeger\internal\exporters\storageexporter\exporter_test.go
and cmd\jaeger\internal\extension\jaegerstorage\factory.go

component id assigned correctly then what can be the reason for its failure?
Screenshot 2025-02-21 000747

@yurishkuro
Copy link
Member

the tests are creating extension like this:

	samplingExtension, err := extensionFactory.Create(
		context.Background(),
		extension.Settings{
			TelemetrySettings: component.TelemetrySettings{
				Logger:         zap.L(),
				TracerProvider: nooptrace.NewTracerProvider(),
			},
		},
		cfg,
	)

meanwhile extension.Settings has a component.ID field that we're not initializing. Perhaps there was a new requirement added in OTEL for that component.ID to not be empty.

@yurishkuro
Copy link
Member

This fixes some of the tests:

@@ -35,6 +35,7 @@ func makeStorageExtension(t *testing.T, memstoreName string) component.Host {
        storageExtension, err := extensionFactory.Create(
                context.Background(),
                extension.Settings{
+                       ID:                jaegerstorage.ID,
                        TelemetrySettings: telemetrySettings,
                },
                &jaegerstorage.Config{
@@ -61,6 +62,7 @@ func makeRemoteSamplingExtension(t *testing.T, cfg component.Config) component.H
        samplingExtension, err := extensionFactory.Create(
                context.Background(),
                extension.Settings{
+                       ID: remotesampling.ID,
                        TelemetrySettings: component.TelemetrySettings{
                                Logger:         zap.L(),
                                TracerProvider: nooptrace.NewTracerProvider(),

@AnmolxSingh
Copy link
Contributor Author

AnmolxSingh commented Feb 20, 2025

Screenshot 2025-02-21 021239

Screenshot 2025-02-21 021115

for this error I found a random string generator

@yurishkuro
Copy link
Member

it complains about the type, not the random string part

Signed-off-by: Yuri Shkuro <[email protected]>
@yurishkuro yurishkuro enabled auto-merge February 20, 2025 22:42
@yurishkuro yurishkuro added this pull request to the merge queue Feb 20, 2025
Merged via the queue into jaegertracing:main with commit 8d031d8 Feb 20, 2025
56 checks passed
@AnmolxSingh AnmolxSingh deleted the otel branch February 21, 2025 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants