[grpc][v2] Use standard OTLP receiver for gRPC storage write path #7065

mahadzaryab1 · 2025-04-22T02:13:11Z

Which problem is this PR solving?

Resolves Use Standard OTLP Receiver for Remote Storage Backend Write Path #7060

Description of the changes

This PR adds a new config called writer to the gRPC storage to allow a separate client configuration for the write path. This new configuration was utilized in the integration tests to point the collector utilizing gRPC storage to the receiver in the remote storage backend.

How was this change tested?

Updated integration tests

Checklist

I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
I have signed all commits
I have added unit tests for the new functionality
I have run lint and test steps successfully
- for jaeger: make lint test
- for jaeger-ui: npm run lint and npm run test

Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com>

codecov · 2025-04-22T02:20:22Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.02%. Comparing base (d03e700) to head (f6c265d).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7065      +/-   ##
==========================================
- Coverage   96.03%   96.02%   -0.01%     
==========================================
  Files         355      355              
  Lines       20987    20993       +6     
==========================================
+ Hits        20155    20159       +4     
- Misses        626      628       +2     
  Partials      206      206

Flag	Coverage Δ
badger_v1	`9.96% <0.00%> (-0.01%)`	⬇️
badger_v2	`2.06% <0.00%> (-0.01%)`	⬇️
cassandra-4.x-v1-manual	`14.99% <0.00%> (-0.01%)`	⬇️
cassandra-4.x-v2-auto	`2.05% <0.00%> (-0.01%)`	⬇️
cassandra-4.x-v2-manual	`2.05% <0.00%> (-0.01%)`	⬇️
cassandra-5.x-v1-manual	`14.99% <0.00%> (-0.01%)`	⬇️
cassandra-5.x-v2-auto	`2.05% <0.00%> (-0.01%)`	⬇️
cassandra-5.x-v2-manual	`2.05% <0.00%> (-0.01%)`	⬇️
elasticsearch-6.x-v1	`19.93% <0.00%> (-0.02%)`	⬇️
elasticsearch-7.x-v1	`20.01% <0.00%> (-0.02%)`	⬇️
elasticsearch-8.x-v1	`20.18% <0.00%> (-0.02%)`	⬇️
elasticsearch-8.x-v2	`2.17% <0.00%> (+0.10%)`	⬆️
grpc_v1	`11.52% <0.00%> (-0.01%)`	⬇️
grpc_v2	`2.06% <0.00%> (-0.01%)`	⬇️
kafka-3.x-v1	`10.24% <0.00%> (-0.01%)`	⬇️
kafka-3.x-v2	`2.06% <0.00%> (-0.01%)`	⬇️
memory_v2	`2.06% <0.00%> (-0.01%)`	⬇️
opensearch-1.x-v1	`20.06% <0.00%> (-0.02%)`	⬇️
opensearch-2.x-v1	`20.06% <0.00%> (-0.02%)`	⬇️
opensearch-2.x-v2	`2.06% <0.00%> (-0.01%)`	⬇️
query	`2.06% <0.00%> (-0.01%)`	⬇️
tailsampling-processor	`0.56% <0.00%> (-0.01%)`	⬇️
unittests	`94.79% <100.00%> (-0.01%)`	⬇️

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.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com>

yurishkuro · 2025-04-22T23:19:49Z

internal/storage/v2/grpc/config.go

+
+	// TODO: add documentation
+	Writer                  configgrpc.ClientConfig `mapstructure:"writer"`
+	configgrpc.ClientConfig `mapstructure:",squash"`


keep at the top, it's the primary endpoint

yurishkuro · 2025-04-22T23:21:09Z

internal/storage/v2/grpc/config.go

 	exporterhelper.TimeoutConfig `mapstructure:",squash"`
+
+	// TODO: add documentation


Suggested change

// TODO: add documentation

// Writer allows overriding the endpoint for writes, e.g. to an OTLP receiver.

// If not defined the main endpoint is used for reads and writes.

yurishkuro · 2025-04-22T23:21:57Z

internal/storage/v2/grpc/factory.go

@@ -52,17 +52,24 @@ func NewFactory(
 		config: cfg,
 	}

+	var writerConfig *configgrpc.ClientConfig


Any reason not to copy by value? Sharing could have side effects.

Suggested change

var writerConfig *configgrpc.ClientConfig

var writerConfig configgrpc.ClientConfig

yurishkuro · 2025-04-22T23:24:28Z

internal/storage/v2/grpc/handler.go

 )

 type Handler struct {
 	storage.UnimplementedTraceReaderServer
 	storage.UnimplementedDependencyReaderServer
-	ptraceotlp.UnimplementedGRPCServer

 	traceReader tracestore.Reader
 	traceWriter tracestore.Writer


do we need the writer?

what happens with archive storage, does it need the writer?

what happens with archive storage, does it need the writer?

Is there a reason the archive storage would need it? After we did the refactor, there is no difference in archive and regular storage

Archive storage is used to save a trace from the UI. If we're testing query service config with gRPC storage then the archive storage in the query binary must be able to perform a write, which it will do by calling the Write API on port :17272. So yeah that single gRPC server should be serving both read and write endpoints. I don't think we want to have yet another binary with OTLP receiver for that.

NB I don't think the e2e test is actually exercising the "archive trace" workflow, it would be good to add that.

Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com>

…egertracing#7065)  ## Which problem is this PR solving? - Resolves jaegertracing#7060 ## Description of the changes - This PR adds a new config called `writer` to the gRPC storage to allow a separate client configuration for the write path. This new configuration was utilized in the integration tests to point the collector utilizing gRPC storage to the receiver in the remote storage backend. ## How was this change tested? - Updated integration tests ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `npm run lint` and `npm run test` --------- Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com>

…egertracing#7065)  ## Which problem is this PR solving? - Resolves jaegertracing#7060 ## Description of the changes - This PR adds a new config called `writer` to the gRPC storage to allow a separate client configuration for the write path. This new configuration was utilized in the integration tests to point the collector utilizing gRPC storage to the receiver in the remote storage backend. ## How was this change tested? - Updated integration tests ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `npm run lint` and `npm run test` --------- Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com> Signed-off-by: amol-verma-allen <amol.verma@allen.in>

mahadzaryab1 added 3 commits April 21, 2025 21:53

Add New Configuration Option For Write Client

9c9986e

Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com>

Propagate New Config And Fallback To Default

6112433

Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com>

Remove Write Endpoint From Remote Storage Backend Handler

4de7eef

Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com>

mahadzaryab1 added the changelog:exprimental Change to an experimental part of the code label Apr 22, 2025

Fix Ports

9e0ca2a

Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com>

yurishkuro reviewed Apr 22, 2025

View reviewed changes

mahadzaryab1 added 3 commits April 22, 2025 19:26

Address Feedback

c25d5d4

Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com>

Bring Back Write Endpoint

777e10c

Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com>

Add Unit Test

f6c265d

Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com>

mahadzaryab1 changed the title ~~[WIP] Use standard OTLP receiver for gRPC storage write path~~ [grpc][v2] Use standard OTLP receiver for gRPC storage write path Apr 23, 2025

mahadzaryab1 marked this pull request as ready for review April 23, 2025 02:35

mahadzaryab1 requested a review from a team as a code owner April 23, 2025 02:35

mahadzaryab1 requested review from joe-elliott and yurishkuro April 23, 2025 02:35

dosubot bot added area/storage v2 labels Apr 23, 2025

yurishkuro approved these changes Apr 23, 2025

View reviewed changes

mahadzaryab1 added this pull request to the merge queue Apr 23, 2025

Merged via the queue into jaegertracing:main with commit fe2ad2e Apr 23, 2025
60 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[grpc][v2] Use standard OTLP receiver for gRPC storage write path #7065

[grpc][v2] Use standard OTLP receiver for gRPC storage write path #7065

Uh oh!

mahadzaryab1 commented Apr 22, 2025 •

edited

Loading

Uh oh!

codecov bot commented Apr 22, 2025 •

edited

Loading

Uh oh!

yurishkuro Apr 22, 2025

Uh oh!

yurishkuro Apr 22, 2025

Uh oh!

yurishkuro Apr 22, 2025

Uh oh!

yurishkuro Apr 22, 2025

Uh oh!

yurishkuro Apr 22, 2025

Uh oh!

mahadzaryab1 Apr 22, 2025 •

edited

Loading

Uh oh!

yurishkuro Apr 23, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

		exporterhelper.TimeoutConfig `mapstructure:",squash"`

		// TODO: add documentation

	// TODO: add documentation
	// Writer allows overriding the endpoint for writes, e.g. to an OTLP receiver.
	// If not defined the main endpoint is used for reads and writes.

	var writerConfig *configgrpc.ClientConfig
	var writerConfig configgrpc.ClientConfig

[grpc][v2] Use standard OTLP receiver for gRPC storage write path #7065

[grpc][v2] Use standard OTLP receiver for gRPC storage write path #7065

Uh oh!

Conversation

mahadzaryab1 commented Apr 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which problem is this PR solving?

Description of the changes

How was this change tested?

Checklist

Uh oh!

codecov bot commented Apr 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

yurishkuro Apr 22, 2025

Choose a reason for hiding this comment

Uh oh!

yurishkuro Apr 22, 2025

Choose a reason for hiding this comment

Uh oh!

yurishkuro Apr 22, 2025

Choose a reason for hiding this comment

Uh oh!

yurishkuro Apr 22, 2025

Choose a reason for hiding this comment

Uh oh!

yurishkuro Apr 22, 2025

Choose a reason for hiding this comment

Uh oh!

mahadzaryab1 Apr 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yurishkuro Apr 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mahadzaryab1 commented Apr 22, 2025 •

edited

Loading

codecov bot commented Apr 22, 2025 •

edited

Loading

mahadzaryab1 Apr 22, 2025 •

edited

Loading

yurishkuro Apr 23, 2025 •

edited

Loading