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

otlpreceiver: performance issue about otlpreceiver invoke proto.Size slow path #6947

Closed
hanjm opened this issue Jan 14, 2023 · 5 comments
Closed
Labels
bug Something isn't working

Comments

@hanjm
Copy link
Member

hanjm commented Jan 14, 2023

Describe the bug
The otlpreceiver consume lots of cpu for proto.Size, marshal to buffer, get buffer size.
Here is the pprof:
image
image
image
Code:

// Enable OpenTelemetry observability plugin.
opts = append(opts, grpc.WithUnaryInterceptor(otelgrpc.UnaryClientInterceptor(otelOpts...)))

Steps to reproduce
Run collector with otlp receiver.

What did you expect to see?
Use fast path to get size of proto message.
What did you see instead?
slow path to get size of proto message, marshal to buffer, get buffer size.

What version did you use?
v0.59.0

What config did you use?

Environment
OS: Linux
Compiler: Go 1.19

Additional context

@hanjm hanjm added the bug Something isn't working label Jan 14, 2023
@gbbr
Copy link
Member

gbbr commented Jan 17, 2023

Hi Jimmie!

I'm not sure familiar with this code, but maybe I can help? What is the fast path that you were thinking of? How should it work instead?

Also, is this the right repository for this issue? It seems like the problematic code lives in open-telemetry/opentelemetry-go, and the problematic call lives in golang/protobuf.

@hanjm
Copy link
Member Author

hanjm commented Jan 17, 2023

hi @gbbr , thank your reply.

What is the fast path that you were thinking of? How should it work instead?

I think it shoud try get size from asserting interface Size() int, then try to use proto.Size

is this the right repository for this issue?

opentelemetry-collector use gogoprotobuf, but opentelemetry-go-contrib use golang/profobuf. it happens in otlpreceiver, so i create issue in this repository, I have open a issue to opentelemetry-go-contrib repository just now.

@gbbr
Copy link
Member

gbbr commented Jan 17, 2023

I think it shoud try get size from asserting interface Size() int, then try to use proto.Size

Thanks for clarifying. I think that makes sense. I would update the "What did you expect to see?" in the Description of the issue to say that.

@atoulme
Copy link
Contributor

atoulme commented Dec 14, 2023

Is this fixed with open-telemetry/opentelemetry-go-contrib#3168?

@hanjm
Copy link
Member Author

hanjm commented Dec 17, 2023

fixed. close it .

@hanjm hanjm closed this as completed Dec 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants