search

moby / buildkit

concurrent, cache-efficient, and Dockerfile-agnostic builder toolkit
https://github.com/moby/moby/issues/34227
Apache License 2.0
7.85k stars 1.09k forks source link

Feature Request: OTEL: (allow to) instrumentalize solver.cacheManager #4443

Open azr opened 7 months ago

azr commented 7 months ago

Hello !

I think it would be nice be able to OTEL measure what the cacheManager does during the import phase, by passing the context around.

I see two options here:

1/ (my preferred option), to break the api of the following functions, to give them a ctx as first arg, and then pass it down:

https://github.com/moby/buildkit/blob/17b2854b5305fbf16fd4d2c3920323d88e55644b/solver/types.go#L254 https://github.com/moby/buildkit/blob/17b2854b5305fbf16fd4d2c3920323d88e55644b/solver/cachestorage.go#L16-L29

2/ store & pass down the context received in the NewCacheManager here https://github.com/moby/buildkit/blob/b598b52e5507b6e46a50800ba3c35a9749cbfdab/solver/cachemanager.go#L22

( Resolve, of the export takes a context already and this would be easy to do )

Would it be okay for me to do this ?

Cheers !

Related: #4429

tonistiigi commented 7 months ago

I think this would be too verbose for the regular trace and make it less readable. If we can keep it opt-in then it would probably be fine.

I'm not 100% sure if these calls can always be associated with specific build requests. OTEL tracer atm is always initialized with a build request coming in to the daemon. In that case regular daemon logs (in trace level) may be better.

azr commented 7 months ago

I think this would be too verbose for the regular trace and make it less readable. If we can keep it opt-in then it would probably be fine.

Yes, totally, I think I would not add any metrics code to the existing & local cache things. But simply add it to the ones that do remote calls even. Would you make that also optional still ?

I'm not 100% sure if these calls can always be associated with specific build requests. OTEL tracer atm is always initialized with a build request coming in to the daemon. In that case regular daemon logs (in trace level) may be better.

In our case we have a buildkit deamon per build so this should be okay. Also, this would probably not always be on. Do you mean that the deamon already logs this or that I should probably add log lines that would trace for us ? If you mean the second, is there a code example somewhere doing thigs ? Thanks ! ( trying to figure this out by myself too )