[grpc][v2] Implement `FindTraceIDs` Call in gRPC reader for remote storage api v2 #6858

mahadzaryab1 · 2025-03-16T20:51:40Z

Which problem is this PR solving?

Towards Implement GRPC Client For Interacting With Remote Storage #6789

Description of the changes

This PR implements the GetOperations call in the gRPC v2 API client
In the process, we also had to add a helper to convert a pcommon.Map to the proto representation []*KeyValue.

How was this change tested?

Added unit 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-03-16T21:00:02Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.13%. Comparing base (712c1bc) to head (e439cd9).
Report is 63 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6858      +/-   ##
==========================================
+ Coverage   96.03%   96.13%   +0.09%     
==========================================
  Files         367      341      -26     
  Lines       20831    19738    -1093     
==========================================
- Hits        20005    18975    -1030     
+ Misses        630      577      -53     
+ Partials      196      186      -10

Flag	Coverage Δ
badger_v1	`10.20% <ø> (+0.32%)`	⬆️
badger_v2	`2.12% <ø> (+0.06%)`	⬆️
cassandra-4.x-v1-manual	`15.36% <ø> (+0.48%)`	⬆️
cassandra-4.x-v2-auto	`2.11% <ø> (+0.06%)`	⬆️
cassandra-4.x-v2-manual	`2.11% <ø> (+0.06%)`	⬆️
cassandra-5.x-v1-manual	`15.36% <ø> (+0.48%)`	⬆️
cassandra-5.x-v2-auto	`2.11% <ø> (+0.06%)`	⬆️
cassandra-5.x-v2-manual	`2.11% <ø> (+0.06%)`	⬆️
elasticsearch-6.x-v1	`20.15% <ø> (+0.60%)`	⬆️
elasticsearch-7.x-v1	`20.23% <ø> (+0.60%)`	⬆️
elasticsearch-8.x-v1	`20.40% <ø> (+0.61%)`	⬆️
elasticsearch-8.x-v2	`2.12% <ø> (+0.06%)`	⬆️
grpc_v1	`11.28% <ø> (+0.35%)`	⬆️
grpc_v2	`8.25% <ø> (+0.26%)`	⬆️
kafka-3.x-v1	`10.51% <ø> (+0.33%)`	⬆️
kafka-3.x-v2	`2.12% <ø> (+0.06%)`	⬆️
memory_v2	`2.12% <ø> (+0.06%)`	⬆️
opensearch-1.x-v1	`20.28% <ø> (+0.60%)`	⬆️
opensearch-2.x-v1	`20.28% <ø> (+0.60%)`	⬆️
opensearch-2.x-v2	`2.12% <ø> (+0.06%)`	⬆️
tailsampling-processor	`0.57% <ø> (+0.01%)`	⬆️
unittests	`94.92% <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.

internal/storage/v2/grpc/tracereader.go

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

yurishkuro · 2025-03-17T02:37:14Z

internal/jptrace/attributes.go

@@ -17,20 +14,3 @@ const (
 	// e.g. proto, thrift, json.
 	FormatAttribute = "@jaeger@format"
 )
-
-func PcommonMapToPlainMap(attributes pcommon.Map) map[string]string {


I would've kept it here. Creating too many top packages at internal just creates clutter, and in this case it's a new package for just a couple of functions.

@yurishkuro I had to do this because of circular dependencies. proto-gen uses jptrace.TracesData and this package needs to use proto-gen for converting to the key-value list. Let me know if you prefer a different approach. We could create a top-level jotel and create sub-packages jptrace and jpcommon in there. Or alternatively move the TracesData out of jptrace.

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

yurishkuro · 2025-03-19T04:18:37Z

internal/jpcommon/map.go

+
+func PcommonMapToPlainMap(attributes pcommon.Map) map[string]string {
+	mapAttributes := make(map[string]string)
+	attributes.Range(func(k string, v pcommon.Value) bool {


nit: I recommend using the new iterator methods, I think it will be attributes.All()

unless they still haven't merged those, I thought they did in the last release

internal/storage/v2/api/tracestore/reader.go

internal/storage/v2/grpc/tracereader.go

internal/jpcommon/map.go

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

…orage api v2 (jaegertracing#6858)  ## Which problem is this PR solving? - Towards jaegertracing#6789 ## Description of the changes - This PR implements the GetOperations call in the gRPC v2 API client - In the process, we also had to add a helper to convert a `pcommon.Map` to the proto representation `[]*KeyValue`. ## How was this change tested? - Added unit 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>

…orage api v2 (jaegertracing#6858)  ## Which problem is this PR solving? - Towards jaegertracing#6789 ## Description of the changes - This PR implements the GetOperations call in the gRPC v2 API client - In the process, we also had to add a helper to convert a `pcommon.Map` to the proto representation `[]*KeyValue`. ## How was this change tested? - Added unit 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 March 16, 2025 16:48

Implement FindTraceIDs

bb55c5b

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

Add Nolint

49e29a7

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

Add Unit Tests

9be14d1

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

mahadzaryab1 added the changelog:bugfix-or-minor-feature label Mar 16, 2025

mahadzaryab1 requested a review from yurishkuro March 16, 2025 20:51

mahadzaryab1 requested a review from a team as a code owner March 16, 2025 20:51

dosubot bot added area/storage v2 labels Mar 16, 2025

Remove Whitespace

90f54ae

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

mahadzaryab1 marked this pull request as draft March 16, 2025 20:53

yurishkuro reviewed Mar 16, 2025

View reviewed changes

internal/storage/v2/grpc/tracereader.go Outdated Show resolved Hide resolved

yurishkuro reviewed Mar 16, 2025

View reviewed changes

internal/storage/v2/grpc/tracereader.go Outdated Show resolved Hide resolved

yurishkuro approved these changes Mar 16, 2025

View reviewed changes

mahadzaryab1 added 5 commits March 16, 2025 20:42

Create New jpcommon Package

5eb4a58

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

Add Helper To Convert Map To Key Value List

c0be5ec

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

Add Unit Tests

0603478

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

Use Helpers

1949251

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

Add Unit Tests

0668825

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

yurishkuro reviewed Mar 17, 2025

View reviewed changes

mahadzaryab1 added 2 commits March 18, 2025 22:06

Fix Error Message

ef917ea

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

Fix Unit Test

33c1deb

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

mahadzaryab1 marked this pull request as ready for review March 19, 2025 02:14

mahadzaryab1 added 2 commits March 18, 2025 22:26

Add Test For Bytes

788012f

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

Add Missing Code Path

1f1d3cc

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

yurishkuro reviewed Mar 19, 2025

View reviewed changes

internal/storage/v2/api/tracestore/reader.go Outdated Show resolved Hide resolved

yurishkuro reviewed Mar 19, 2025

View reviewed changes

internal/storage/v2/grpc/tracereader.go Outdated Show resolved Hide resolved

yurishkuro reviewed Mar 19, 2025

View reviewed changes

internal/jpcommon/map.go Outdated Show resolved Hide resolved

Move Helper Inline

d2327c7

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

mahadzaryab1 added 3 commits March 19, 2025 17:56

Move Helper Inline

b45c6e1

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

Revert Changes To attributes.go

78083eb

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

Revert Changes To attributes.go

e439cd9

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

yurishkuro approved these changes Mar 20, 2025

View reviewed changes

yurishkuro added this pull request to the merge queue Mar 20, 2025

Merged via the queue into jaegertracing:main with commit 663e450 Mar 20, 2025
56 checks passed

mahadzaryab1 deleted the grpc-find-trace-id branch April 2, 2025 17:13

mahadzaryab1 added changelog:exprimental Change to an experimental part of the code and removed changelog:bugfix-or-minor-feature labels Apr 2, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[grpc][v2] Implement `FindTraceIDs` Call in gRPC reader for remote storage api v2 #6858

[grpc][v2] Implement `FindTraceIDs` Call in gRPC reader for remote storage api v2 #6858

Uh oh!

mahadzaryab1 commented Mar 16, 2025 •

edited

Loading

Uh oh!

codecov bot commented Mar 16, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

yurishkuro Mar 17, 2025

Uh oh!

mahadzaryab1 Mar 19, 2025

Uh oh!

yurishkuro Mar 19, 2025

Uh oh!

yurishkuro Mar 19, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

[grpc][v2] Implement FindTraceIDs Call in gRPC reader for remote storage api v2 #6858

[grpc][v2] Implement FindTraceIDs Call in gRPC reader for remote storage api v2 #6858

Uh oh!

Conversation

mahadzaryab1 commented Mar 16, 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 Mar 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

yurishkuro Mar 17, 2025

Choose a reason for hiding this comment

Uh oh!

mahadzaryab1 Mar 19, 2025

Choose a reason for hiding this comment

Uh oh!

yurishkuro Mar 19, 2025

Choose a reason for hiding this comment

Uh oh!

yurishkuro Mar 19, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

[grpc][v2] Implement `FindTraceIDs` Call in gRPC reader for remote storage api v2 #6858

[grpc][v2] Implement `FindTraceIDs` Call in gRPC reader for remote storage api v2 #6858

mahadzaryab1 commented Mar 16, 2025 •

edited

Loading

codecov bot commented Mar 16, 2025 •

edited

Loading