gophercloud icon indicating copy to clipboard operation
gophercloud copied to clipboard

Introduce context usage in the service client

Open pkenchap opened this issue 2 years ago • 7 comments

Context usage in the gopher cloud gives more request handling and goroutines working on a request typically needs access to request-specific values such as the identity of the end user, authorization tokens, and the request’s deadline. if we enable the context which provide easy integration external tools .

why we are proposing to have a context in the client is to integration of **Opentelemetry (**https://github.com/open-telemetry/opentelemetry-go) in the Gophercloud sdk (https://github.com/gophercloud/gophercloud) . provides the tracing of the API can be monitored with the timelines, which gives immense use for the performance test for calculation individual api time taken for the operation. Also gives a nice UI based graphs using the JaegerUI .

pkenchap avatar Jul 05 '23 05:07 pkenchap

Thank you for reporting your first issue! Be sure that we will be looking at it, but keep in mind this sometimes takes a while. Please let the maintainers know if your issue has not got enough attention after a few days. If any doubt, please consult our issue tutorial.

github-actions[bot] avatar Jul 05 '23 05:07 github-actions[bot]

What I understand from your inquiry is that Gophercloud should provide a more ergonomic way to handle both tracing and cancellation, and that both these requirements beautifully match with the capabilities of Go's context.Context.

If that's what you are saying, then I agree 100%.

I am surprised though that you mention the service client.

Currently, Gophercloud accepts a context.Context in the ProviderClient. That context is then injected in every HTTP call to the OpenStack API. I find the current model problematic, as the context is provider-scoped rather than call-scoped.

I would gladly switch to a model where each Gophercloud call accepts a context.Context, like this:

pages, err := servers.ListContext(ctx, client, nil).AllPages()

Additional DoXxxContext functions could be implemented in each Gophercloud package in a backwards-compatible manner at the cost of implementing this ugly context-merging code I was experimenting with in this PR.

What are your thoughts and requirements? Why are you thinking at a service-client context instead of a per-call context?

pierreprinetti avatar Jul 05 '23 09:07 pierreprinetti

we may need to pass the context parameter to functions in the service_client.go file . For example : Adding the ctx context.Context to the function which has these REST API endpoints . func (client *ServiceClient) Get(ctx context.Context, url string, JSONResponse interface{}, opts *RequestOpts) (*http.Response, error)

https://github.com/gophercloud/gophercloud/blob/496af9bd7c8768494f8eb55b32e8b72fd0e78bba/service_client.go#L71

pkenchap avatar Jul 05 '23 09:07 pkenchap

Before we talk implementation, I want to understand your requirements.

Do you specifically need some state to be attached to a long-lived service_client, or perhaps what you really want is a context that is narrowly scoped to a single call? I would expect tracing to require each call to the API to have a specific ID.

pierreprinetti avatar Jul 05 '23 10:07 pierreprinetti

Yes , you were right , Provider client expects the context also this context need to provide from the user how initiate the call . which comes from the requests -> service -> provider . in between the tracing REST API call we need context to see the flow . Also for tracing we can use the open telemetry package and jaeger package from graphical projections . Which looks like the below screenshots with jaeger integration .

Screenshot 2023-07-05 at 3 33 46 PM

pkenchap avatar Jul 05 '23 10:07 pkenchap

Please take a look at the PR which gives more info - https://github.com/gophercloud/gophercloud/pull/2686

service client will call the Request method in the Provider_client it does not contain a context , only when we call the do_request we can pass the context to the provider_client so it can be used in the api call.

https://github.com/pkenchap/gophercloud/blob/cf659998b191737a8e9116dcf2f31918635558af/service_client.go#L153

if we use the https://github.com/pkenchap/gophercloud/blob/cf659998b191737a8e9116dcf2f31918635558af/provider_client.go#L363

do request method for the context in the service client then we can provide the context to the provider_client .

pkenchap avatar Jul 11 '23 05:07 pkenchap

Note for self: a follow-up work may be to accept a X-OpenStack-Request-ID as a context.Context value, see https://github.com/gophercloud/gophercloud/issues/2428

pierreprinetti avatar Aug 13 '23 10:08 pierreprinetti