[jaeger-v2] Implement empty service name sanitizer for OTLP by mahadzaryab1 · Pull Request #6077 · jaegertracing/jaeger

mahadzaryab1 · 2024-10-12T02:23:57Z

Which problem is this PR solving?

Towards Transform v1 sanitizers into processors or something else #5545

Description of the changes

Created a new Sanitizer type for sanitizing traces in OTLP format
Implemented the empty service name sanitizer; I'll open a separate PR for the UTF-8 sanitizer

How was this change tested?

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: yarn lint and yarn test

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

codecov · 2024-10-12T02:32:18Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.92%. Comparing base (563f3b0) to head (f3deafa).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6077      +/-   ##
==========================================
- Coverage   96.93%   96.92%   -0.01%     
==========================================
  Files         349      351       +2     
  Lines       16646    16675      +29     
==========================================
+ Hits        16135    16162      +27     
- Misses        328      329       +1     
- Partials      183      184       +1

Flag	Coverage Δ
badger_v1	`7.98% <ø> (ø)`
badger_v2	`1.84% <ø> (ø)`
cassandra-4.x-v1	`15.75% <ø> (ø)`
cassandra-4.x-v2	`1.77% <ø> (ø)`
cassandra-5.x-v1	`15.75% <ø> (ø)`
cassandra-5.x-v2	`1.77% <ø> (ø)`
elasticsearch-6.x-v1	`18.92% <ø> (-0.03%)`	⬇️
elasticsearch-7.x-v1	`19.01% <ø> (+0.01%)`	⬆️
elasticsearch-8.x-v1	`19.19% <ø> (-0.02%)`	⬇️
elasticsearch-8.x-v2	`1.84% <ø> (ø)`
grpc_v1	`9.36% <ø> (ø)`
grpc_v2	`7.12% <ø> (ø)`
kafka-v1	`9.68% <ø> (ø)`
kafka-v2	`1.84% <ø> (ø)`
memory_v2	`1.83% <ø> (ø)`
opensearch-1.x-v1	`19.05% <ø> (ø)`
opensearch-2.x-v1	`19.05% <ø> (ø)`
opensearch-2.x-v2	`1.84% <ø> (ø)`
tailsampling-processor	`0.49% <ø> (ø)`
unittests	`95.73% <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.

yurishkuro · 2024-10-12T02:32:43Z

do not capitalize all words in PR titles, only the first one (i.e. normal sentence case).

cmd/jaeger/internal/exporters/storageexporter/exporter.go

cmd/jaeger/internal/exporters/storageexporter/sanitizer/emptyservicename.go

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

cmd/jaeger/internal/exporters/storageexporter/sanitizer/emptyservicename.go

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

cmd/jaeger/internal/exporters/storageexporter/exporter.go

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

…acing#6077) ## Which problem is this PR solving? - Towards jaegertracing#5545 ## Description of the changes - Created a new `Sanitizer` type for sanitizing traces in OTLP format - Implemented the empty service name sanitizer; I'll open a separate PR for the UTF-8 sanitizer ## How was this change tested? - 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`: `yarn lint` and `yarn test` --------- Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com> Signed-off-by: Saumya Shah <saumyabshah90@gmail.com>

## Which problem is this PR solving? - Resolves #5545 ## Description of the changes - Continuation of #6077 to implement the UTF-8 sanitizer to operate on OTLP data. ## How was this change tested? - 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`: `yarn lint` and `yarn test` --------- Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com>

…acing#6077) ## Which problem is this PR solving? - Towards jaegertracing#5545 ## Description of the changes - Created a new `Sanitizer` type for sanitizing traces in OTLP format - Implemented the empty service name sanitizer; I'll open a separate PR for the UTF-8 sanitizer ## How was this change tested? - 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`: `yarn lint` and `yarn test` --------- Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com>

## Which problem is this PR solving? - Resolves jaegertracing#5545 ## Description of the changes - Continuation of jaegertracing#6077 to implement the UTF-8 sanitizer to operate on OTLP data. ## How was this change tested? - 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`: `yarn lint` and `yarn test` --------- Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com>

## Which problem is this PR solving? - Resolves part of #7261 ## Description of the changes - When introduced: PR #67 - when replaced: - First Replacement - PR #3631 - modern OTLP implementation - PR #6077 - final location - PR #7158 - Current State: - Old-dead `cmd/collector/app/sanitizer/service_name_sanitizer.go` - Legacy-used `cmd/collector/app/sanitizer/empty_service_name_sanitizer.go` - Modern-used `internal/jptrace/sanitizer/emptyservicename.go` - No prod usage - Files to remove: - `cmd/collector/app/sanitizer/service_name_sanitizer.go` - `cmd/collector/app/sanitizer/service_name_sanitizer_test.go` ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [ ] 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: Parship Chowdhury <i.am.parship@gmail.com> Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>

mahadzaryab1 added 7 commits October 11, 2024 19:38

Create Interface For Initializing Sanitizers

232c3fd

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

Add Documentation For Standard Sanitizers Function

648058e

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

Implement Empty Service Name Sanitizer

0a90741

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

Add Sanitizer To Exporter

f4c1bcd

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

Add package_test

24e1a6b

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

Add Test For New Chained Sanitizer

b65efe7

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

Run Formatter

a52a4b0

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

yurishkuro reviewed Oct 12, 2024

View reviewed changes

cmd/jaeger/internal/exporters/storageexporter/exporter.go Outdated Show resolved Hide resolved

mahadzaryab1 changed the title ~~[jaeger-v2] Implement Empty Service Name Sanitizer To Work With OTLP~~ [jaeger-v2] Implement empty service name sanitizer to work with otlp Oct 12, 2024

yurishkuro reviewed Oct 12, 2024

View reviewed changes

cmd/jaeger/internal/exporters/storageexporter/sanitizer/emptyservicename.go Outdated Show resolved Hide resolved

Move Sanitizer Outside of ExportersDirectory

252f710

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

yurishkuro reviewed Oct 12, 2024

View reviewed changes

cmd/jaeger/internal/exporters/storageexporter/sanitizer/emptyservicename.go Outdated Show resolved Hide resolved

mahadzaryab1 added 2 commits October 11, 2024 22:38

Remove Debugging Statements

24f1df9

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

Add Test For New Standard Sanitizers

f032c20

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

mahadzaryab1 marked this pull request as ready for review October 12, 2024 02:47

mahadzaryab1 requested a review from a team as a code owner October 12, 2024 02:47

mahadzaryab1 requested a review from jkowall October 12, 2024 02:47

dosubot bot added the v2 label Oct 12, 2024

mahadzaryab1 and others added 3 commits October 11, 2024 22:57

Add Check For Non String Service Name

3fca5a8

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

Merge branch 'main' into v2-sanitizer

4a28cf4

Use Switch Statement

b12dbee

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

yurishkuro changed the title ~~[jaeger-v2] Implement empty service name sanitizer to work with otlp~~ [jaeger-v2] Implement empty service name sanitizer for OTLP Oct 12, 2024

yurishkuro reviewed Oct 12, 2024

View reviewed changes

cmd/jaeger/internal/exporters/storageexporter/exporter.go Show resolved Hide resolved

mahadzaryab1 added 3 commits October 11, 2024 23:36

Add Check For Sanitizer In exporter_test

7ccd378

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

Fix Typo

ce9ce70

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

Rename Function From Sanitize To Func

f3deafa

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

yurishkuro approved these changes Oct 12, 2024

View reviewed changes

yurishkuro added the changelog:experimental Change to an experimental part of the code label Oct 12, 2024

yurishkuro merged commit 4beead7 into jaegertracing:main Oct 12, 2024

mahadzaryab1 mentioned this pull request Oct 12, 2024

[jaeger-v2] Implement UTF-8 sanitizer for OTLP #6078

Merged

4 tasks

mahadzaryab1 deleted the v2-sanitizer branch October 31, 2024 22:35

This was referenced Jul 22, 2025

Remove dead code #7261

Closed

Remove sanitizer/service_name_sanitizer (dead code) #7366

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[jaeger-v2] Implement empty service name sanitizer for OTLP#6077

[jaeger-v2] Implement empty service name sanitizer for OTLP#6077
yurishkuro merged 16 commits intojaegertracing:mainfrom
mahadzaryab1:v2-sanitizer

mahadzaryab1 commented Oct 12, 2024

Uh oh!

codecov bot commented Oct 12, 2024 •

edited

Loading

Uh oh!

yurishkuro commented Oct 12, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

mahadzaryab1 commented Oct 12, 2024

Which problem is this PR solving?

Description of the changes

How was this change tested?

Checklist

Uh oh!

codecov bot commented Oct 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

yurishkuro commented Oct 12, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov bot commented Oct 12, 2024 •

edited

Loading