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

[feature request] performance: otlp http json support jsoniter/sonic for better performance #4817

Closed
hanjm opened this issue Jan 28, 2022 · 6 comments
Closed

[feature request] performance: otlp http json support jsoniter/sonic for better performance #4817

hanjm opened this issue Jan 28, 2022 · 6 comments
Labels
area:receiver Stale

Comments

@hanjm
Copy link
Member

hanjm commented Jan 28, 2022 •
edited
Loading

Is your feature request related to a problem? Please describe.
I foud collector (which use otlp http json) cpu profile cost largely in json unmarshal, I know protobuf have better preformance but json is more frendly to frontend.
image
Describe the solution you'd like
A clear and concise description of what you want to happen.
Could we have a better performance via jsoniter or sonic?
Like prometheus/loki/elastic-apm project does.
https://github.com/prometheus/prometheus/blob/main/web/api/v1/api.go#L196
https://github.com/grafana/loki/blob/main/pkg/storage/chunk/json_helpers.go#L13
https://github.com/elastic/apm-server/blob/master/model/modeldecoder/nullable/nullable.go#L30
If the feature request is reasonable, I think we could add config field to oltpreceiver to choose another unmarshaller, or just make a new component.
Describe alternatives you've considered

Additional context
@hanjm hanjm mentioned this issue Jan 29, 2022
Closed
6 tasks
@sethAmazon
Copy link
Contributor

I think this code is owned by the upstream collector. https://github.com/open-telemetry/opentelemetry-collector/tree/main/receiver/otlpreceiver

@sethAmazon
Copy link
Contributor

We use a custom jsonpb.Marshaler. I'm not sure how this work but if you want to take a look go here. https://github.com/sethAmazon/opentelemetry-collector/blob/main/receiver/otlpreceiver/internal/marshal_jsonpb.go

@Aneurysm9 Aneurysm9 transferred this issue from open-telemetry/opentelemetry-collector-contrib Feb 7, 2022
@alolita
Copy link
Member

alolita commented Feb 7, 2022

@open-telemetry/collector-maintainers this is related to OTLP receiver perf - please comment.

@tigrannajaryan
Copy link
Member

I think better performance is always welcome, however given that OTLP/JSON is unstable I would prefer personally not to spend time on improving the performance just yet. I don't mind if someone else wants to work on this.

hanjm added a commit to hanjm/opentelemetry-collector that referenced this issue Mar 13, 2022
@hanjm
Implement unmarshal traces with jsoniter, make 40x faster (open-telem…
b7379aa 
…etry#4817)

goos: darwin
goarch: amd64
pkg: go.opentelemetry.io/collector/model/otlp
cpu: Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
BenchmarkTracesJSONUnmarshal-12            10000            102486 ns/op           78030 B/op       1151 allocs/op
BenchmarkTracesJSONiterUnmarshal-12       512875              2431 ns/op            4002 B/op        185 allocs/op
hanjm added a commit to hanjm/opentelemetry-collector that referenced this issue Mar 13, 2022
@hanjm
Implement unmarshal traces with jsoniter, make 40x faster (open-telem…
bfa3fcd 
…etry#4817)

goos: darwin
goarch: amd64
pkg: go.opentelemetry.io/collector/model/otlp
cpu: Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
BenchmarkTracesJSONUnmarshal-12            10000            102486 ns/op           78030 B/op       1151 allocs/op
BenchmarkTracesJSONiterUnmarshal-12       512875              2431 ns/op            4002 B/op        185 allocs/op
@hanjm hanjm mentioned this issue Mar 13, 2022
Merged
hanjm added a commit to hanjm/opentelemetry-collector that referenced this issue Mar 13, 2022
@hanjm
Implement unmarshal traces with jsoniter, make 40x faster (open-telem…
6ff4ba6 
…etry#4817)

goos: darwin
goarch: amd64
pkg: go.opentelemetry.io/collector/model/otlp
cpu: Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
BenchmarkTracesJSONUnmarshal-12            10000            102486 ns/op           78030 B/op       1151 allocs/op
BenchmarkTracesJSONiterUnmarshal-12       512875              2431 ns/op            4002 B/op        185 allocs/op
hanjm added a commit to hanjm/opentelemetry-collector that referenced this issue Mar 13, 2022
@hanjm
Implement unmarshal traces with jsoniter, make 40x faster (open-telem…
f4bfa8b 
…etry#4817)

goos: darwin
goarch: amd64
pkg: go.opentelemetry.io/collector/model/otlp
cpu: Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
BenchmarkTracesJSONUnmarshal-12            10000            102486 ns/op           78030 B/op       1151 allocs/op
BenchmarkTracesJSONiterUnmarshal-12       512875              2431 ns/op            4002 B/op        185 allocs/op
hanjm added a commit to hanjm/opentelemetry-collector that referenced this issue Mar 13, 2022
@hanjm
Implement unmarshal traces with jsoniter, make 40x faster than jsonpb (…
0ec4ace 
…open-telemetry#4817)

goos: darwin
goarch: amd64
pkg: go.opentelemetry.io/collector/model/otlp
cpu: Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
BenchmarkTracesJSONUnmarshal-12            10000            102486 ns/op           78030 B/op       1151 allocs/op
BenchmarkTracesJSONiterUnmarshal-12       512875              2431 ns/op            4002 B/op        185 allocs/op
hanjm added a commit to hanjm/opentelemetry-collector that referenced this issue Mar 13, 2022
@hanjm
Implement unmarshal traces with jsoniter, make 40x faster than jsonpb (…
a47dea8 
…open-telemetry#4817)

goos: darwin
goarch: amd64
pkg: go.opentelemetry.io/collector/model/otlp
cpu: Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
BenchmarkTracesJSONUnmarshal-12            10000            102486 ns/op           78030 B/op       1151 allocs/op
BenchmarkTracesJSONiterUnmarshal-12       512875              2431 ns/op            4002 B/op        185 allocs/op
@hanjm
Copy link
Member Author

hanjm commented Mar 14, 2022

Hi @sethAmazon @tigrannajaryan , I submit PR #4986 to implement unmarshal traces with jsoniter for better performance in the OTLP HTTP JSON case.

bogdandrutu pushed a commit that referenced this issue May 9, 2022
@hanjm
Implement unmarshal traces with jsoniter, make 35x faster than jsonpb(#…
dfc8d70 
…4817) (#4986)

* Implement unmarshal traces with jsoniter, make 40x faster than jsonpb.

Signed-off-by: Jimmie Han <[email protected]>

* ptrace: json unmarshaller use defer

* Use jsoniter unmarshaller as default trace unmarshaler.
Update unit test style design.

* Add kvlist support
@atingchen
Copy link
Contributor

Hi @sethAmazon @tigrannajaryan , I submit PR #5433 to implement unmarshal metrics with jsoniter for better performance in the OTLP HTTP JSON case.

@atingchen atingchen mentioned this issue Sep 6, 2022
Closed
@github-actions github-actions bot added the Stale label May 26, 2024
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jun 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area:receiver Stale
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@alolita @tigrannajaryan @hanjm @atingchen @sethAmazon