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

feat: Add OTEL tracer to metricsstore reader component #4595

Merged
merged 13 commits into from
Jul 29, 2023
Merged

feat: Add OTEL tracer to metricsstore reader component #4595

merged 13 commits into from
Jul 29, 2023

Conversation

afzal442
Copy link
Contributor

@afzal442 afzal442 commented Jul 23, 2023 •
edited
Loading

Which problem is this PR solving?

Short description of the changes

  • Replaces OT tracer with OTEL tracer to support jtracer pkg

@afzal442
adds otel tracer to metrics reader component
1faf91e 
Signed-off-by: Afzal Ansari <afzal442@gmail.com>
@afzal442 afzal442 requested a review from a team as a code owner July 23, 2023 06:51
@afzal442 afzal442 requested a review from vprithvi July 23, 2023 06:51
@codecov
Copy link

codecov bot commented Jul 23, 2023 •
edited
Loading

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.02% ⚠️

Comparison is base (2c1bf07) 97.08% compared to head (2d298f3) 97.06%.
Report is 3 commits behind head on main.

❗ Current head 2d298f3 differs from pull request most recent head 7d4624b. Consider uploading reports for the commit 7d4624b to get more accurate results

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #4595      +/-   ##
==========================================
- Coverage   97.08%   97.06%   -0.02%     
==========================================
  Files         301      301              
  Lines       17857    17862       +5     
==========================================
+ Hits        17336    17338       +2     
- Misses        418      420       +2     
- Partials      103      104       +1
Flag Coverage Δ
unittests 97.06% <100.00%> (-0.02%) ⬇️

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

Files Changed Coverage Δ
plugin/metrics/prometheus/factory.go 100.00% <100.00%> (ø)
plugin/metrics/prometheus/metricsstore/reader.go 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

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

yurishkuro
yurishkuro reviewed Jul 23, 2023
View reviewed changes
plugin/metrics/prometheus/factory.go Outdated Show resolved Hide resolved
plugin/metrics/prometheus/metricsstore/reader.go Outdated Show resolved Hide resolved
plugin/metrics/prometheus/metricsstore/reader.go Outdated Show resolved Hide resolved
plugin/metrics/prometheus/metricsstore/reader.go Outdated Show resolved Hide resolved
plugin/metrics/prometheus/metricsstore/reader.go Outdated Show resolved Hide resolved
plugin/metrics/prometheus/metricsstore/reader_test.go Outdated Show resolved Hide resolved
@afzal442
updates the tracer to use global tp
faaecd4 
Signed-off-by: Afzal Ansari <afzal442@gmail.com>
yurishkuro
yurishkuro reviewed Jul 24, 2023
View reviewed changes
pkg/jtracer/jtracer.go Outdated Show resolved Hide resolved
afzal442 added 2 commits July 25, 2023 11:46
@afzal442
adds condition to jt closer
ec7d45d 
Signed-off-by: Afzal Ansari <afzal442@gmail.com>
@afzal442
adds tp to main and creates tp in test
ffcc85e 
Signed-off-by: Afzal Ansari <afzal442@gmail.com>
@afzal442
adds exporter.GetSpan() to test recorded spans
4852cec 
Signed-off-by: Afzal Ansari <afzal442@gmail.com>
afzal442 added 2 commits July 26, 2023 09:44
@afzal442
fixes the span creation for query
a6b2884 
Signed-off-by: Afzal Ansari <afzal442@gmail.com>
@afzal442
fixes the shutdown issue
09b3fbf 
Signed-off-by: Afzal Ansari <afzal442@gmail.com>
@afzal442 afzal442 changed the title [WIP] Add OTEL tracer to metrics reader component feat: Add OTEL tracer to metrics reader component Jul 26, 2023
@afzal442 afzal442 changed the title feat: Add OTEL tracer to metrics reader component feat: Add OTEL tracer to metricsstore reader component Jul 26, 2023
@afzal442
validates the no. of spans reported
a0dcb35 
Signed-off-by: Afzal Ansari <afzal442@gmail.com>
@afzal442 afzal442 force-pushed the replace-with-otel-tp branch from a8b1c3e to c7c9c62 Compare July 27, 2023 19:09
@afzal442
reorders the defer to execute the close
e279905 
Signed-off-by: Afzal Ansari <afzal442@gmail.com>
@afzal442 afzal442 force-pushed the replace-with-otel-tp branch from c7c9c62 to e279905 Compare July 27, 2023 19:20
@afzal442
fixes spans reported err
4319e62 
Signed-off-by: Afzal Ansari <afzal442@gmail.com>
@afzal442
moves defer to its obj creation
3295970 
Signed-off-by: Afzal Ansari <afzal442@gmail.com>
@afzal442 afzal442 requested a review from yurishkuro July 28, 2023 15:14
@afzal442 afzal442 force-pushed the replace-with-otel-tp branch from 47cb9d6 to d822929 Compare July 28, 2023 17:29
@afzal442
reorders the assert in the sequence call
abeeeb6 
Signed-off-by: Afzal Ansari <afzal442@gmail.com>
@afzal442 afzal442 force-pushed the replace-with-otel-tp branch from d822929 to abeeeb6 Compare July 28, 2023 17:32
@afzal442 afzal442 force-pushed the replace-with-otel-tp branch 2 times, most recently from 1e647a3 to 6fdb455 Compare July 28, 2023 19:18
@afzal442 @sbin64
sets the span status
7d4624b 
Signed-off-by: Afzal Ansari <afzal442@gmail.com>
@sbin64 sbin64 force-pushed the replace-with-otel-tp branch from 2d298f3 to 7d4624b Compare July 29, 2023 12:16
@yurishkuro yurishkuro merged commit 287562c into jaegertracing:main Jul 29, 2023
@sbin64 sbin64 deleted the replace-with-otel-tp branch July 29, 2023 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yurishkuro yurishkuro yurishkuro approved these changes

@vprithvi vprithvi Awaiting requested review from vprithvi vprithvi is a code owner automatically assigned from jaegertracing/jaeger-maintainers

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@afzal442 @yurishkuro