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

Add telemetry #289

Open
polRk opened this issue Jul 11, 2024 · 15 comments
Open

Add telemetry #289

polRk opened this issue Jul 11, 2024 · 15 comments
Labels
enhancement New feature or request

Comments

@polRk
Copy link

polRk commented Jul 11, 2024

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

@polRk polRk added the enhancement New feature or request label Jul 11, 2024
@slvrtrn
Copy link
Contributor

slvrtrn commented Jul 11, 2024

Can you please provide a more detailed description of your use case?
Do you maybe have a good example of another language client library doing that?

@polRk
Copy link
Author

polRk commented Jul 12, 2024

Can you please provide a more detailed description of your use case?

Collect queries count, quries duration. monitoring performance clickhouse library itself, expose tracings

https://opentelemetry.io.

Do you maybe have a good example of another language client library doing that?

http://effect.website

@maxdeichmann
Copy link

maxdeichmann commented Aug 7, 2024

@slvrtrn id also be highly interested. Prisma provides a tracing feature including metrics and spans. I think an otel integration would be perfect here.
Eventually, i want to be able to track which clickhouse queries are slow.

@slvrtrn
Copy link
Contributor

slvrtrn commented Aug 7, 2024

Can this be implemented as a standalone add-on (maybe as a package in this repo, like packages/client-node-otel), or do you need more detailed metrics from within the client internals?

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.

@maxdeichmann
Copy link

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 functio

I think having this as its own dependency is totally fine.

@polRk
Copy link
Author

polRk commented Aug 13, 2024

The reason I am asking is that it's a bit concerning to add a ~1.9M dependency

These are constants for naming by convention - see @opentelemetry/semantic-conventions,

@polRk
Copy link
Author

polRk commented Aug 13, 2024

Can this be implemented as a standalone add-on (maybe as a package in this repo, like packages/client-node-otel), or do you need more detailed metrics from within the client internals?

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 client ch <- (reading metrics in a opentelemetry format) <- sdk opentelemetry <- ..processing

The Clickhouse client must be a metricProducer, https://opentelemetry.io/docs/specs/otel/metrics/sdk/#metricproducer

@slvrtrn
Copy link
Contributor

slvrtrn commented Aug 14, 2024

This makes sense; however, I am currently busy with other projects. Would you like to create a PR for this feature?

@polRk
Copy link
Author

polRk commented Aug 14, 2024

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)

@slvrtrn
Copy link
Contributor

slvrtrn commented Aug 14, 2024

@polRk, DM me in the community Slack; I could do a short intro.

@constb
Copy link

constb commented Sep 18, 2024

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…

@slvrtrn
Copy link
Contributor

slvrtrn commented Sep 18, 2024

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.

@constb
Copy link

constb commented Sep 19, 2024

@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 :)

@maxdeichmann
Copy link

@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.

@slvrtrn
Copy link
Contributor

slvrtrn commented Oct 10, 2024

@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:

Rows: [
  { number: '0' },
  { number: '1' },
  { number: '2' },
  { number: '3' },
  { number: '4' }
]
Summary: {"read_rows":"5","read_bytes":"40","written_rows":"0","written_bytes":"0","total_rows_to_read":"5","result_rows":"0","result_bytes":"0","elapsed_ns":"2100708"}

However, please note that the summary might be incorrect without the wait_end_of_query=1 setting (see ClickHouse/ClickHouse#12122), which should be used with caution (see e.g., ClickHouse/ClickHouse#46426 (comment))

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.

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

4 participants