search

googleapis / google-cloud-go

Google Cloud Client Libraries for Go.
https://cloud.google.com/go/docs/reference
Apache License 2.0
3.74k stars 1.29k forks source link

storage: create lots of span name. #1573

Closed ukai closed 4 years ago

ukai commented 5 years ago

cloud.google.com/go/storage.NewClient use google.golang.org/api/transport/http.NewClient, which sets up ochttp.Transport. ochttp.Transport default will create span with request's Path as span name.

If zpages is enabled, all span names will be kept.

If client access lots of objects, request's Path differs for each object, so it consumes lots of memory to keep span for each object.

better to use fewer span name for storage access.

tritone commented 5 years ago

Hi @ukai , I don't think this is something that we will be able to fix in the storage client. Spans are generated automatically for all outgoing requests. If you're concerned about memory usage when zpages is enabled, I would file a bug with that project (https://github.com/census-instrumentation/opencensus-go).

MatthewDolan commented 5 years ago

Hi @tritone - I think the problem is that the storage client doesn't allow overriding the settings for the OC http transport or disabling the OC http transport entirely.

It does seem that the combination of the the default HTTP middleware and the memory consumption model of zpages don't work well (and maybe an issue should be filed against OC for that), but if the gcs client allowed passing options, that would be enough of an escape hatch for resolving the issue.

I put together a small change that might resolve the issue for us. This would allow a consumer of the storage client to override the default OC http transport behavior.

https://github.com/googleapis/google-api-go-client/compare/master...MatthewDolan:dolan-2019-10-01-oc-options

I haven't had the chance to install Gerritt yet so I haven't submitted the patch for review, but I can probably do that next week.

ukai commented 5 years ago

yeah, I'd like to have an option to configure FormatSpanName https://github.com/census-instrumentation/opencensus-go/blob/master/plugin/ochttp/client.go#L54

tritone commented 5 years ago

Thanks very much for clarifying the issue, and for your proposed solution! I'll have to do some more research to make sure this will work but feel free to create a code review whenever it's convenient for you.