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

Inject trace_configs as an argument to opentelemetry aiohttp_client._instrument instrumentation #1073

Closed
nemoshlag opened this issue Apr 28, 2022 · 3 comments · Fixed by #1079

Comments

@nemoshlag
Copy link
Member

Is your feature request related to a problem?

I would like to add data to a span based on on_request_chunk_sent event. Using the request/response hooks doesn't provide a good solution for my use case since it's "too late" in the timeline.

Describe the solution you'd like

I would like to pass trace_configs (or a single trace_config) to aiohttp_client opentelemetry instrumentation. It will allow me to specify which events I would like to "catch" and will allow more customization solving many use-cases that the request/response hooks cannot.

Describe alternatives you've considered

I've doubled the instrument_init method and set it up with my own trace_configs, bypassing opentelemetry implementation.

Additional context
If this issue will be approved I would like to contribute my solution, thanks

@nemoshlag nemoshlag changed the title inject trace_configs as an argument to opentelemetry aiohttp_client._instrument instrumentation Inject trace_configs as an argument to opentelemetry aiohttp_client._instrument instrumentation Apr 28, 2022
@srikanthccv
Copy link
Member

@nemoshlag Can you share little example of what this change/solution looks like and what are you trying to achieve?

@nemoshlag
Copy link
Member Author

nemoshlag commented Apr 28, 2022

Hi @srikanthccv sure, pseudo usage from client side:

`def trace_config(...)-> aiohttp.TraceConfig:
async def on_request_chunk_sent(...):
# do stuff

AioHttpClientInstrumentor._instument(tracer_provider, request_hook, response_hook, trace_configs=trace_config)`

And on opentelemetry instrumentation side:

`def _instrument(tracer_provider: TracerProvider = None, args, kwargs):
def instrumented_init(wrapped, instance, args, kwargs) -> None:
trace_configs = list(kwargs.get("trace_configs") or ())

    trace_config = create_trace_config(
        tracer_provider=tracer_provider,
        request_hook=kwargs.get("request_hook"),
        response_hook=kwargs.get("response_hook"),
    )

    trace_configs.append(trace_config)

    ...`

@srikanthccv
Copy link
Member

Feel free to send a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants