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

Support uploading traces to UI in OpenTelemetry format (OTLP JSON) #5155

Merged
merged 14 commits into from
Feb 14, 2024

Conversation

NavinShrinivas
Copy link
Contributor

@NavinShrinivas NavinShrinivas commented Jan 30, 2024

Which problem is this PR solving?

Description of the changes

  • Incorporates changes from previous draft PR review
  • Adds middleware in frontend as well

How was this change tested?

  • Unit tests and supplying valid and invalid OTLP traces using insomnia to test the API

Checklist

Signed-off-by: Navin Shrinivas <karupal2002@gmail.com>

partial work for issue jaegertracing#4949
@NavinShrinivas NavinShrinivas changed the title Uploading jaeger traces Uploading OTLP traces Jan 30, 2024
Copy link
Member

@yurishkuro yurishkuro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there should be no UI submodule changes in this PR.

@NavinShrinivas
Copy link
Contributor Author

there should be no UI submodule changes in this PR.

How do I make pr's for jaeger-ui changes?

@yurishkuro
Copy link
Member

in jaeger-ui repository

Signed-off-by: Navin Shrinivas <karupal2002@gmail.com>
@NavinShrinivas
Copy link
Contributor Author

NavinShrinivas commented Jan 30, 2024

hey @yurishkuro, I have implemented the group by traceID and I have excluded jaeger-ui changes and added some tests. I'll make a pr to jaeger-ui only after the backend is confirmed

Copy link
Member

@yurishkuro yurishkuro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and I have excluded jaeger-ui changes

can you submit another PR for UI changes? The functionality is not useful without both of them.

cmd/query/app/apiv3/package_test.go Outdated Show resolved Hide resolved
model/trace.go Outdated Show resolved Hide resolved
cmd/query/app/apiv3/otlp_translator.go Outdated Show resolved Hide resolved
cmd/query/app/apiv3/otlp_translator.go Outdated Show resolved Hide resolved
cmd/query/app/http_handler.go Outdated Show resolved Hide resolved
@NavinShrinivas
Copy link
Contributor Author

NavinShrinivas commented Jan 31, 2024

and I have excluded jaeger-ui changes

can you submit another PR for UI changes? The functionality is not useful without both of them.

I wont be able to link the issue in the jaeger-ui repo, is that alright? Or do you want me to push a seperate pr changing the commit version tracked in this repo for jaeger-ui?

I've made a PR anyways : jaegertracing/jaeger-ui#2145

Signed-off-by: Navin Shrinivas <karupal2002@gmail.com>
@yurishkuro
Copy link
Member

I wont be able to link the issue in the jaeger-ui repo, is that alright?

Yes you can, see how I just edited UI PR description - you just include URL to the ticket and GitHub links them

Signed-off-by: Navin Shrinivas <karupal2002@gmail.com>
@NavinShrinivas NavinShrinivas marked this pull request as ready for review February 1, 2024 06:45
Signed-off-by: Navin Shrinivas <karupal2002@gmail.com>
@NavinShrinivas
Copy link
Contributor Author

NavinShrinivas commented Feb 7, 2024

Im unable to add the required label for ci/cd

Copy link

codecov bot commented Feb 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (8ae2e05) 94.38% compared to head (463556c) 94.41%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5155      +/-   ##
==========================================
+ Coverage   94.38%   94.41%   +0.02%     
==========================================
  Files         331      332       +1     
  Lines       19156    19189      +33     
==========================================
+ Hits        18081    18117      +36     
+ Misses        886      884       -2     
+ Partials      189      188       -1     
Flag Coverage Δ
cassandra-3.x 25.58% <ø> (ø)
cassandra-4.x 25.58% <ø> (ø)
elasticsearch-5.x 19.86% <ø> (+0.01%) ⬆️
elasticsearch-6.x 19.85% <ø> (-0.02%) ⬇️
elasticsearch-7.x 19.98% <ø> (ø)
elasticsearch-8.x 20.08% <ø> (+0.01%) ⬆️
grpc-badger 19.48% <ø> (ø)
kafka 14.09% <ø> (ø)
opensearch-1.x 20.00% <ø> (ø)
opensearch-2.x 19.98% <ø> (-0.02%) ⬇️
unittests 92.18% <100.00%> (+0.02%) ⬆️

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.

@NavinShrinivas
Copy link
Contributor Author

I just saw the missing tests, from what I can see, golang testify lets you mock functions that are methods of a struct, here the model2otlp functions are one-off separate functions. Would it be okay to include them as part of aH (APIHandler), allowing me to mock them like tracesByID function? Or is there some alternative method I'm missing out on?

@yurishkuro yurishkuro added the changelog:new-feature Change that should be called out as new feature in CHANGELOG label Feb 9, 2024
Signed-off-by: Navin Shrinivas <karupal2002@gmail.com>
Signed-off-by: Navin Shrinivas <karupal2002@gmail.com>
Copy link
Member

@yurishkuro yurishkuro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found one issue on the UI side: jaegertracing/jaeger-ui#2145 (comment)

cmd/query/app/http_handler_test.go Outdated Show resolved Hide resolved
cmd/query/app/http_handler_test.go Outdated Show resolved Hide resolved
cmd/query/app/http_handler_test.go Show resolved Hide resolved
cmd/query/app/http_handler_test.go Outdated Show resolved Hide resolved
cmd/query/app/http_handler_test.go Outdated Show resolved Hide resolved
cmd/query/app/http_handler_test.go Outdated Show resolved Hide resolved
Signed-off-by: Navin Shrinivas <karupal2002@gmail.com>
Signed-off-by: Yuri Shkuro <github@ysh.us>
@@ -0,0 +1,110 @@
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reformatted per cat file | jq .

out, err := json.MarshalIndent(obj, "", " ")
require.NoError(t, err)
return out
}
Copy link
Member

@yurishkuro yurishkuro Feb 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you were right the first time about employing marshaling to ensure consistent formatting. I changed the expected fixture to be readable - also added a 2nd trace to test a branch in the translator

@yurishkuro yurishkuro marked this pull request as ready for review February 14, 2024 02:08
Signed-off-by: Yuri Shkuro <github@ysh.us>
@yurishkuro yurishkuro merged commit b60a7fc into jaegertracing:main Feb 14, 2024
34 of 35 checks passed
dmitryax pushed a commit to open-telemetry/opentelemetry-collector-contrib that referenced this pull request Mar 12, 2024
…31709)

[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
|
[github.com/jaegertracing/jaeger](https://togithub.com/jaegertracing/jaeger)
| `v1.54.0` -> `v1.55.0` |
[![age](https://developer.mend.io/api/mc/badges/age/go/github.com%2fjaegertracing%2fjaeger/v1.55.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/go/github.com%2fjaegertracing%2fjaeger/v1.55.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/go/github.com%2fjaegertracing%2fjaeger/v1.54.0/v1.55.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/go/github.com%2fjaegertracing%2fjaeger/v1.54.0/v1.55.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

> [!WARNING]
> Some dependencies could not be looked up. Check the Dependency
Dashboard for more information.

---

### Release Notes

<details>
<summary>jaegertracing/jaeger
(github.com/jaegertracing/jaeger)</summary>

###
[`v1.55.0`](https://togithub.com/jaegertracing/jaeger/releases/tag/v1.55.0):
Release 1.55.0

[Compare
Source](https://togithub.com/jaegertracing/jaeger/compare/v1.54.0...v1.55.0)

##### Backend Changes

##### ✨ New Features:

- Support uploading traces to UI in OpenTelemetry format (OTLP/JSON)
([@&#8203;NavinShrinivas](https://togithub.com/NavinShrinivas) in
[#&#8203;5155](https://togithub.com/jaegertracing/jaeger/pull/5155))
- Add Elasticsearch storage support for adaptive sampling
([@&#8203;Pushkarm029](https://togithub.com/Pushkarm029) in
[#&#8203;5158](https://togithub.com/jaegertracing/jaeger/pull/5158))

##### 🐞 Bug fixes, Minor Improvements:

- Add the `print-config` subcommand
([@&#8203;gmafrac](https://togithub.com/gmafrac) in
[#&#8203;5200](https://togithub.com/jaegertracing/jaeger/pull/5200))
- Return more detailed errors from ES storage
([@&#8203;yurishkuro](https://togithub.com/yurishkuro) in
[#&#8203;5209](https://togithub.com/jaegertracing/jaeger/pull/5209))
- Bump go version ([@&#8203;yurishkuro](https://togithub.com/yurishkuro)
in [#&#8203;5180](https://togithub.com/jaegertracing/jaeger/pull/5180))

##### 🚧 Experimental Features:

- \[jaeger-v2] Add support for gRPC storarge
([@&#8203;james-ryans](https://togithub.com/james-ryans) in
[#&#8203;5228](https://togithub.com/jaegertracing/jaeger/pull/5228))
- \[jaeger-v2] Add support for Elasticsearch
([@&#8203;akagami-harsh](https://togithub.com/akagami-harsh) in
[#&#8203;5152](https://togithub.com/jaegertracing/jaeger/pull/5152))

##### 📊 UI Changes

- UI pinned to version
[1.39.0](https://togithub.com/jaegertracing/jaeger-ui/blob/main/CHANGELOG.md#v1390-2024-03-04).

##### 👏 New Contributors

- [@&#8203;h4shk4t](https://togithub.com/h4shk4t) made their first
contribution in
[jaegertracing/jaeger#5162
- [@&#8203;NavinShrinivas](https://togithub.com/NavinShrinivas) made
their first contribution in
[jaegertracing/jaeger#5155
- [@&#8203;prakrit55](https://togithub.com/prakrit55) made their first
contribution in
[jaegertracing/jaeger#5197
- [@&#8203;gmafrac](https://togithub.com/gmafrac) made their first
contribution in
[jaegertracing/jaeger#5200

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "on tuesday" (UTC), Automerge - At any
time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/open-telemetry/opentelemetry-collector-contrib).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4yMzguMSIsInVwZGF0ZWRJblZlciI6IjM3LjIzOC4xIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: opentelemetrybot <107717825+opentelemetrybot@users.noreply.github.com>
DougManton pushed a commit to DougManton/opentelemetry-collector-contrib that referenced this pull request Mar 13, 2024
…pen-telemetry#31709)

[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
|
[github.com/jaegertracing/jaeger](https://togithub.com/jaegertracing/jaeger)
| `v1.54.0` -> `v1.55.0` |
[![age](https://developer.mend.io/api/mc/badges/age/go/github.com%2fjaegertracing%2fjaeger/v1.55.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/go/github.com%2fjaegertracing%2fjaeger/v1.55.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/go/github.com%2fjaegertracing%2fjaeger/v1.54.0/v1.55.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/go/github.com%2fjaegertracing%2fjaeger/v1.54.0/v1.55.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

> [!WARNING]
> Some dependencies could not be looked up. Check the Dependency
Dashboard for more information.

---

### Release Notes

<details>
<summary>jaegertracing/jaeger
(github.com/jaegertracing/jaeger)</summary>

###
[`v1.55.0`](https://togithub.com/jaegertracing/jaeger/releases/tag/v1.55.0):
Release 1.55.0

[Compare
Source](https://togithub.com/jaegertracing/jaeger/compare/v1.54.0...v1.55.0)

##### Backend Changes

##### ✨ New Features:

- Support uploading traces to UI in OpenTelemetry format (OTLP/JSON)
([@&open-telemetry#8203;NavinShrinivas](https://togithub.com/NavinShrinivas) in
[#&open-telemetry#8203;5155](https://togithub.com/jaegertracing/jaeger/pull/5155))
- Add Elasticsearch storage support for adaptive sampling
([@&open-telemetry#8203;Pushkarm029](https://togithub.com/Pushkarm029) in
[#&open-telemetry#8203;5158](https://togithub.com/jaegertracing/jaeger/pull/5158))

##### 🐞 Bug fixes, Minor Improvements:

- Add the `print-config` subcommand
([@&open-telemetry#8203;gmafrac](https://togithub.com/gmafrac) in
[#&open-telemetry#8203;5200](https://togithub.com/jaegertracing/jaeger/pull/5200))
- Return more detailed errors from ES storage
([@&open-telemetry#8203;yurishkuro](https://togithub.com/yurishkuro) in
[#&open-telemetry#8203;5209](https://togithub.com/jaegertracing/jaeger/pull/5209))
- Bump go version ([@&open-telemetry#8203;yurishkuro](https://togithub.com/yurishkuro)
in [#&open-telemetry#8203;5180](https://togithub.com/jaegertracing/jaeger/pull/5180))

##### 🚧 Experimental Features:

- \[jaeger-v2] Add support for gRPC storarge
([@&open-telemetry#8203;james-ryans](https://togithub.com/james-ryans) in
[#&open-telemetry#8203;5228](https://togithub.com/jaegertracing/jaeger/pull/5228))
- \[jaeger-v2] Add support for Elasticsearch
([@&open-telemetry#8203;akagami-harsh](https://togithub.com/akagami-harsh) in
[#&open-telemetry#8203;5152](https://togithub.com/jaegertracing/jaeger/pull/5152))

##### 📊 UI Changes

- UI pinned to version
[1.39.0](https://togithub.com/jaegertracing/jaeger-ui/blob/main/CHANGELOG.md#v1390-2024-03-04).

##### 👏 New Contributors

- [@&open-telemetry#8203;h4shk4t](https://togithub.com/h4shk4t) made their first
contribution in
[jaegertracing/jaeger#5162
- [@&open-telemetry#8203;NavinShrinivas](https://togithub.com/NavinShrinivas) made
their first contribution in
[jaegertracing/jaeger#5155
- [@&open-telemetry#8203;prakrit55](https://togithub.com/prakrit55) made their first
contribution in
[jaegertracing/jaeger#5197
- [@&open-telemetry#8203;gmafrac](https://togithub.com/gmafrac) made their first
contribution in
[jaegertracing/jaeger#5200

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "on tuesday" (UTC), Automerge - At any
time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/open-telemetry/opentelemetry-collector-contrib).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4yMzguMSIsInVwZGF0ZWRJblZlciI6IjM3LjIzOC4xIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: opentelemetrybot <107717825+opentelemetrybot@users.noreply.github.com>
XinRanZhAWS pushed a commit to XinRanZhAWS/opentelemetry-collector-contrib that referenced this pull request Mar 13, 2024
…pen-telemetry#31709)

[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
|
[github.com/jaegertracing/jaeger](https://togithub.com/jaegertracing/jaeger)
| `v1.54.0` -> `v1.55.0` |
[![age](https://developer.mend.io/api/mc/badges/age/go/github.com%2fjaegertracing%2fjaeger/v1.55.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/go/github.com%2fjaegertracing%2fjaeger/v1.55.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/go/github.com%2fjaegertracing%2fjaeger/v1.54.0/v1.55.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/go/github.com%2fjaegertracing%2fjaeger/v1.54.0/v1.55.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

> [!WARNING]
> Some dependencies could not be looked up. Check the Dependency
Dashboard for more information.

---

### Release Notes

<details>
<summary>jaegertracing/jaeger
(github.com/jaegertracing/jaeger)</summary>

###
[`v1.55.0`](https://togithub.com/jaegertracing/jaeger/releases/tag/v1.55.0):
Release 1.55.0

[Compare
Source](https://togithub.com/jaegertracing/jaeger/compare/v1.54.0...v1.55.0)

##### Backend Changes

##### ✨ New Features:

- Support uploading traces to UI in OpenTelemetry format (OTLP/JSON)
([@&open-telemetry#8203;NavinShrinivas](https://togithub.com/NavinShrinivas) in
[#&open-telemetry#8203;5155](https://togithub.com/jaegertracing/jaeger/pull/5155))
- Add Elasticsearch storage support for adaptive sampling
([@&open-telemetry#8203;Pushkarm029](https://togithub.com/Pushkarm029) in
[#&open-telemetry#8203;5158](https://togithub.com/jaegertracing/jaeger/pull/5158))

##### 🐞 Bug fixes, Minor Improvements:

- Add the `print-config` subcommand
([@&open-telemetry#8203;gmafrac](https://togithub.com/gmafrac) in
[#&open-telemetry#8203;5200](https://togithub.com/jaegertracing/jaeger/pull/5200))
- Return more detailed errors from ES storage
([@&open-telemetry#8203;yurishkuro](https://togithub.com/yurishkuro) in
[#&open-telemetry#8203;5209](https://togithub.com/jaegertracing/jaeger/pull/5209))
- Bump go version ([@&open-telemetry#8203;yurishkuro](https://togithub.com/yurishkuro)
in [#&open-telemetry#8203;5180](https://togithub.com/jaegertracing/jaeger/pull/5180))

##### 🚧 Experimental Features:

- \[jaeger-v2] Add support for gRPC storarge
([@&open-telemetry#8203;james-ryans](https://togithub.com/james-ryans) in
[#&open-telemetry#8203;5228](https://togithub.com/jaegertracing/jaeger/pull/5228))
- \[jaeger-v2] Add support for Elasticsearch
([@&open-telemetry#8203;akagami-harsh](https://togithub.com/akagami-harsh) in
[#&open-telemetry#8203;5152](https://togithub.com/jaegertracing/jaeger/pull/5152))

##### 📊 UI Changes

- UI pinned to version
[1.39.0](https://togithub.com/jaegertracing/jaeger-ui/blob/main/CHANGELOG.md#v1390-2024-03-04).

##### 👏 New Contributors

- [@&open-telemetry#8203;h4shk4t](https://togithub.com/h4shk4t) made their first
contribution in
[jaegertracing/jaeger#5162
- [@&open-telemetry#8203;NavinShrinivas](https://togithub.com/NavinShrinivas) made
their first contribution in
[jaegertracing/jaeger#5155
- [@&open-telemetry#8203;prakrit55](https://togithub.com/prakrit55) made their first
contribution in
[jaegertracing/jaeger#5197
- [@&open-telemetry#8203;gmafrac](https://togithub.com/gmafrac) made their first
contribution in
[jaegertracing/jaeger#5200

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "on tuesday" (UTC), Automerge - At any
time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/open-telemetry/opentelemetry-collector-contrib).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4yMzguMSIsInVwZGF0ZWRJblZlciI6IjM3LjIzOC4xIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: opentelemetrybot <107717825+opentelemetrybot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:new-feature Change that should be called out as new feature in CHANGELOG
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants