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

Allow custom telemetry handlers #1079

Open
aws-hans-pistor opened this issue Oct 23, 2024 · 2 comments
Open

Allow custom telemetry handlers #1079

aws-hans-pistor opened this issue Oct 23, 2024 · 2 comments
Labels
enhancement New feature or request

Comments

@aws-hans-pistor
Copy link

Tell us more about this new feature.

Background

Different users of the mountpoint client are likely to be interested in different metrics that are relevant to their usecase. In order to support all of them, it would be nice if they could define a custom telemetry handler for their client & then whenever the on_telemetry callback is run, that handler is run as well.

For context, our client downloads different parts of the same object in S3 at high concurrency. We think there is some sort of throttling/slowdown going on somwhere along the request path, but in today's mountpoint we have no way of injecting metrics/logging for requests where the bandwidth is < X MB/s or the duration > Yms. It doesn't really make sense for mountpoint itself to support this usecase, but it also makes sense that users should have the capability to do this themselves

I'll be submitting a PR for this issue, but creating the issue as well if discussion is necessary.

@vladem
Copy link
Contributor

vladem commented Nov 1, 2024

Hey @aws-hans-pistor, thanks for opening the issue and proposing the code change. We'd like to understand the use-case a bit better before proceeding.

I can see that the client already logs some information from telemetry with each HTTP request after it finishes (note that a single call to the client's method, e.g get_object, may result in multiple underlying HTTP requests, e.g. HeadObject + multiple GetObject-s). You need DEBUG level logging to get those logs.

Even more information is logged when the TRACE level is enabled (1, 2).

Ain't this logging sufficient for your use-case? If some information is not extracted from the CRT (e.g. extended request ids) we can consider fixing this for all users of the client.

From the description it looks that you are also looking for a better way of controlling the verbosity of the logs:

way of injecting metrics/logging for requests where the bandwidth is < X MB/s or the duration > Yms

Does it mean that today's TRACE logs are too verbose for your use case and you'd like to have a callback to do custom logging based on data in RequestMetrics? Is the operation: &str required for this? Note that you can extract the type of the HTTP request from RequestMetrics.

@aws-hans-pistor
Copy link
Author

TRACE/DEBUG logging is not really feasible at the scale we're running because we can't control what is logged & what isn't. We need finer grained controlled on what & when we log / emit metrics on.

As for the parameters on the custom logging, that's up to you. I was more or less copying the inputs to the existing telemetry callback, which require the RequestMetrics and the operation that pulled from the Span

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants