Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 http/json protocol #1585
Support http/json protocol #1585
Changes from 29 commits
1482d5c
576a56d
d13ab4b
ed4fc03
587c341
d460bda
da76734
aa74b9c
efa8588
e47a80f
04593ec
277df67
5d5e06e
359babe
8c9577f
f175980
f93058c
46f92c4
0f1a805
6ebea63
e50ac0e
049e96a
d64bda0
ad8322b
e457a85
5222f8d
25b0857
63fb044
bcde320
f9abf24
250fab4
67552f8
765ab99
67d6875
680b191
c6d1817
4ed5158
ebc3cc3
86a89d5
aa2cd05
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Check warning on line 59 in opentelemetry-otlp/src/exporter/http/logs.rs
Codecov / codecov/patch
opentelemetry-otlp/src/exporter/http/logs.rs#L57-L59
Check warning on line 68 in opentelemetry-otlp/src/exporter/http/logs.rs
Codecov / codecov/patch
opentelemetry-otlp/src/exporter/http/logs.rs#L66-L68
Check warning on line 74 in opentelemetry-otlp/src/exporter/http/logs.rs
Codecov / codecov/patch
opentelemetry-otlp/src/exporter/http/logs.rs#L70-L74
Check warning on line 76 in opentelemetry-otlp/src/exporter/http/logs.rs
Codecov / codecov/patch
opentelemetry-otlp/src/exporter/http/logs.rs#L76
Check warning on line 81 in opentelemetry-otlp/src/exporter/http/logs.rs
Codecov / codecov/patch
opentelemetry-otlp/src/exporter/http/logs.rs#L79-L81
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this part. I wonder if we can make the feature additive.
One way I see we can do it is:
protocol
field inOtelHttpClient
build_body
as part of theOtelHttpClient
.Then we just ask users to pass protocol in building pipline(it's already part of the
export_config
. And based on the feature the corresponding protocol may be availableThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we still talking about exporting in one format per signal per pipeline? If so, it should be possible with the above code, no? For e.g I can enable both features
http-json
andhttp-proto
and build 2 trace pipelines that export in each of those formats. Let me know if I'm missing something.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes by tuning the features it's possible to export in either
proto
orjson
. My preference is to ask users to explicitly tell the pipeline which protocol to rather then to config it using features. i.e it should be possible to enable bothhttp-json
andhttp-proto
but choose to usehttp-proto
. I think it current setup doesn't support this.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Understood. Would you want
HttpExporterBuilder
to receive protocol viawith_protocol
?Check warning on line 55 in opentelemetry-otlp/src/exporter/http/metrics.rs
Codecov / codecov/patch
opentelemetry-otlp/src/exporter/http/metrics.rs#L53-L55
Check warning on line 63 in opentelemetry-otlp/src/exporter/http/metrics.rs
Codecov / codecov/patch
opentelemetry-otlp/src/exporter/http/metrics.rs#L61-L63
Check warning on line 70 in opentelemetry-otlp/src/exporter/http/metrics.rs
Codecov / codecov/patch
opentelemetry-otlp/src/exporter/http/metrics.rs#L66-L70
Check warning on line 72 in opentelemetry-otlp/src/exporter/http/metrics.rs
Codecov / codecov/patch
opentelemetry-otlp/src/exporter/http/metrics.rs#L72
Check warning on line 77 in opentelemetry-otlp/src/exporter/http/metrics.rs
Codecov / codecov/patch
opentelemetry-otlp/src/exporter/http/metrics.rs#L75-L77
Check warning on line 92 in opentelemetry-otlp/src/exporter/http/trace.rs
Codecov / codecov/patch
opentelemetry-otlp/src/exporter/http/trace.rs#L92
Check warning on line 97 in opentelemetry-otlp/src/exporter/http/trace.rs
Codecov / codecov/patch
opentelemetry-otlp/src/exporter/http/trace.rs#L95-L97
Check warning on line 153 in opentelemetry-otlp/src/exporter/http/trace.rs
Codecov / codecov/patch
opentelemetry-otlp/src/exporter/http/trace.rs#L153