-
Notifications
You must be signed in to change notification settings - Fork 29
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
Add telemetry #289
Comments
Can you please provide a more detailed description of your use case? |
Collect queries count, quries duration. monitoring performance clickhouse library itself, expose tracings
|
Can this be implemented as a standalone add-on (maybe as a package in this repo, like The reason I am asking is that it's a bit concerning to add a ~1.9M dependency (assuming we need this) to the main client, which is ~500KB on its own, but not everyone needs this functionality. |
I think having this as its own dependency is totally fine. |
These are constants for naming by convention - see @opentelemetry/semantic-conventions, |
Tracing should have a context, this is exactly what needs to be implemented at the library function level. To get metrics, it will be enough if I can read metrics (in opentelemetry format) from the clickhouse client using my opentelemetry/metrics-sdk The Clickhouse client must be a metricProducer, https://opentelemetry.io/docs/specs/otel/metrics/sdk/#metricproducer |
This makes sense; however, I am currently busy with other projects. Would you like to create a PR for this feature? |
Yes, of course, but I need a technical assignment and some introductory information about the clickhouse client library. Where to start (i think about https://github.com/ClickHouse/clickhouse-js/blob/main/packages/client-common/src/config.ts#L217) |
@polRk, DM me in the community Slack; I could do a short intro. |
It would be best if metrics were exposed in a manner that is not dependent on opentelemetry and its conventions. I currently collect metrics for prometheus by wrapping methods of clickhouse client. Nothing special, just counters for the number of queries and the number of errors and a histogram of query execution times. Honestly, it would suffice if clickhouse client was an event emitter sending start/finish/error events… |
I like that this does not require extra dependencies. Then, looks like it should be possible to add necessary wrappers on the application side (we can also provide copy-pasteable examples for OTEL/Prometheus) @constb, can you please provide more details on how exactly you collect metrics in your use case? That could be useful to draft a possible implementation. If it's not shareable in public, you could also DM me in the community Slack. |
@slvrtrn honestly, it's very straightforward. Code is similar to this: const client = createClient(config);
client.exec = wrapWithMetrics(client, client.exec, metrics, clientId);
client.command = wrapWithMetrics(client, client.command, metrics, clientId);
client.query = wrapWithMetrics(client, client.query, metrics, clientId);
client.insert = wrapWithMetrics(client, client.insert, metrics, clientId);
function wrapWithMetrics(
client,
originalMethod,
{ queriesCounter, performanceHistogram, errorsCounter },
clientId,
) {
return (...args: unknown[]) => {
const end = performanceHistogram.startTimer({ clientId });
try {
queriesCounter.inc({ clientId });
return await originalMethod.apply(client, args);
} catch (err: unknown) {
errorsCounter.inc({ clientId });
throw err;
} finally {
end();
}
}
} I typed this code in the comment field, hopefully I didn't miss anything :) |
@slvrtrn is there a way to also get db insights into the telemetry? I would be interested in rows read from disk, amount of threads used, cpu and the like. |
@maxdeichmann, you could use the summary header to a certain extent. For example: import { createClient } from '@clickhouse/client'
void (async () => {
const client = createClient()
const rs = await client.query({
query: 'SELECT number FROM system.numbers LIMIT 5',
format: 'JSONEachRow'
})
const rows = await rs.json<{ number: string }>()
console.log('Rows:', rows)
console.log('Summary:', rs.response_headers['x-clickhouse-summary'])
await client.close()
})() this prints:
However, please note that the summary might be incorrect without the Regarding the advanced stats such as memory and threads usage, we don't get it in the response in any way; you will need to check system.query_log for that; it wouldn't be realistically possible to build this into the client, as the logs are flushed periodically, and not available right away. |
Use case
Collect traces, metrics, logs
Describe the solution you'd like
Use OpenTelemetry for collecting metrics, logs, traces
Describe the alternatives you've considered
Expose metrics as an object via client method (example:
chClient.metrics() # [metric{}, metric{}]
Additional context
The text was updated successfully, but these errors were encountered: