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

[exporter/clickhouse] Can't create metrics table with the new ttl configuration. #29626

Closed
googollee opened this issue Dec 2, 2023 · 8 comments · Fixed by #30209
Closed

[exporter/clickhouse] Can't create metrics table with the new ttl configuration. #29626

googollee opened this issue Dec 2, 2023 · 8 comments · Fixed by #30209
Labels
bug Something isn't working exporter/clickhouse Stale

Comments

@googollee
Copy link

googollee commented Dec 2, 2023

Component(s)

exporter/clickhouse

What happened?

Description

With 0.90.0 collector, when starting with clickhouse exporter, collector can't create metrics tables in clickhouse. It works well with 0.89.0 version, so this bug should be introduced by #29095.

Steps to Reproduce

  • Start a clean clickhouse server locally.
  • docker run --rm -v \pwd`/otel.yaml:/etc/otelcol-contrib/config.yaml --network host ghcr.io/open-telemetry/opentelemetry-collector-releases/opentelemetry-collector-contrib:0.90.0`

Expected Result

The collector image should work well with the configuration.

Actual Result

collector server run finished with error: cannot start pipelines: exec create metrics table sql: code: 47, message: Missing columns: 'Timestamp' while processing query: 'toDateTime(Timestamp) + toIntervalDay(3)', required columns: 'Timestamp' 'Timestamp'

Collector version

v0.90.0

Environment information

Environment

OS: (e.g., "Ubuntu 20.04")
Compiler(if manually compiled): (e.g., "go 14.2")

OpenTelemetry Collector configuration

receivers:
  otlp:
    protocols:
      grpc:
      http:

processors:
  batch:
    timeout: 5s
    send_batch_size: 100000

exporters:
  clickhouse:
    endpoint: clickhouse://localhost:9000?dial_timeout=10s&compress=lz4
    username: default
    password: ""
    database: otel
    ttl: 72h
    logs_table_name: otel_logs
    traces_table_name: otel_traces
    metrics_table_name: otel_metrics
    timeout: 5s
    retry_on_failure:
      enabled: true
      initial_interval: 5s
      max_interval: 30s
      max_elapsed_time: 300s

extensions:
  health_check:
  pprof:
  zpages:

service:
  extensions: [health_check, pprof, zpages]
  pipelines:
    traces:
      receivers: [otlp]
      processors: [batch]
      exporters: [clickhouse]
    metrics:
      receivers: [otlp]
      processors: [batch]
      exporters: [clickhouse]
    logs:
      receivers: [otlp]
      processors: [batch]
      exporters: [clickhouse]

Log output

2023-12-02T16:15:14.022Z	info	service@v0.90.0/telemetry.go:86	Setting up own telemetry...
2023-12-02T16:15:14.026Z	info	service@v0.90.0/telemetry.go:203	Serving Prometheus metrics	{"address": ":8888", "level": "Basic"}
2023-12-02T16:15:14.033Z	info	service@v0.90.0/service.go:148	Starting otelcol-contrib...	{"Version": "0.90.0", "NumCPU": 2}
2023-12-02T16:15:14.034Z	info	extensions/extensions.go:34	Starting extensions...
2023-12-02T16:15:14.034Z	info	extensions/extensions.go:37	Extension is starting...	{"kind": "extension", "name": "zpages"}
2023-12-02T16:15:14.034Z	info	zpagesextension@v0.90.0/zpagesextension.go:53	Registered zPages span processor on tracer provider	{"kind": "extension", "name": "zpages"}
2023-12-02T16:15:14.034Z	info	zpagesextension@v0.90.0/zpagesextension.go:63	Registered Host's zPages	{"kind": "extension", "name": "zpages"}
2023-12-02T16:15:14.037Z	info	zpagesextension@v0.90.0/zpagesextension.go:75	Starting zPages extension	{"kind": "extension", "name": "zpages", "config": {"TCPAddr":{"Endpoint":"localhost:55679"}}}
2023-12-02T16:15:14.037Z	info	extensions/extensions.go:45	Extension started.	{"kind": "extension", "name": "zpages"}
2023-12-02T16:15:14.037Z	info	extensions/extensions.go:37	Extension is starting...	{"kind": "extension", "name": "pprof"}
2023-12-02T16:15:14.043Z	info	pprofextension@v0.90.0/pprofextension.go:60	Starting net/http/pprof server	{"kind": "extension", "name": "pprof", "config": {"TCPAddr":{"Endpoint":"localhost:1777"},"BlockProfileFraction":0,"MutexProfileFraction":0,"SaveToFile":""}}
2023-12-02T16:15:14.043Z	info	extensions/extensions.go:45	Extension started.	{"kind": "extension", "name": "pprof"}
2023-12-02T16:15:14.047Z	info	extensions/extensions.go:37	Extension is starting...	{"kind": "extension", "name": "health_check"}
2023-12-02T16:15:14.047Z	info	healthcheckextension@v0.90.0/healthcheckextension.go:35	Starting health_check extension	{"kind": "extension", "name": "health_check", "config": {"Endpoint":"0.0.0.0:13133","TLSSetting":null,"CORS":null,"Auth":null,"MaxRequestBodySize":0,"IncludeMetadata":false,"ResponseHeaders":null,"Path":"/","ResponseBody":null,"CheckCollectorPipeline":{"Enabled":false,"Interval":"5m","ExporterFailureThreshold":5}}}
2023-12-02T16:15:14.047Z	warn	internal@v0.90.0/warning.go:40	Using the 0.0.0.0 address exposes this server to every network interface, which may facilitate Denial of Service attacks	{"kind": "extension", "name": "health_check", "documentation": "https://github.com/open-telemetry/opentelemetry-collector/blob/main/docs/security-best-practices.md#safeguards-against-denial-of-service-attacks"}
2023-12-02T16:15:14.049Z	info	extensions/extensions.go:45	Extension started.	{"kind": "extension", "name": "health_check"}
2023-12-02T16:15:14.125Z	info	service@v0.90.0/service.go:188	Starting shutdown...
2023-12-02T16:15:14.125Z	info	healthcheck/handler.go:132	Health Check state change	{"kind": "extension", "name": "health_check", "status": "unavailable"}
2023-12-02T16:15:14.125Z	info	extensions/extensions.go:52	Stopping extensions...
2023-12-02T16:15:14.125Z	info	zpagesextension@v0.90.0/zpagesextension.go:98	Unregistered zPages span processor on tracer provider	{"kind": "extension", "name": "zpages"}
2023-12-02T16:15:14.125Z	info	service@v0.90.0/service.go:202	Shutdown complete.
Error: cannot start pipelines: exec create metrics table sql: code: 47, message: Missing columns: 'Timestamp' while processing query: 'toDateTime(Timestamp) + toIntervalDay(3)', required columns: 'Timestamp' 'Timestamp'
2023/12/02 16:15:14 collector server run finished with error: cannot start pipelines: exec create metrics table sql: code: 47, message: Missing columns: 'Timestamp' while processing query: 'toDateTime(Timestamp) + toIntervalDay(3)', required columns: 'Timestamp' 'Timestamp'

Additional context

No response

@googollee googollee added bug Something isn't working needs triage New item requiring triage labels Dec 2, 2023
Copy link
Contributor

github-actions bot commented Dec 2, 2023

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@googollee
Copy link
Author

From the previous comment, I think it should be easy to fix by changing Timestamp to TimeUnix in https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/v0.90.0/exporter/clickhouseexporter/factory.go#L126. But as there is no unit test to check creating SQLs, I'd like to leave it to owners to find a way to test first.

@JaredTan95
Copy link
Member

@Frapschen Can you have a look at it?

@JaredTan95 JaredTan95 removed the needs triage New item requiring triage label Dec 3, 2023
@Frapschen
Copy link
Contributor

Frapschen commented Dec 5, 2023

I'd like to leave it to owners to find a way to test first.

To detect SQL problems like that we need to start a clickhouse server to test the SQLs, I want to use Testcontainers to start a clickhouse container and start our exporter based on it. any thought? @googollee @JaredTan95 @hanjm

@dmitryax
Copy link
Member

dmitryax commented Dec 5, 2023

This should be fixed by #29573. @Frapschen PTAL

@hanjm
Copy link
Member

hanjm commented Dec 5, 2023

I'd like to leave it to owners to find a way to test first.

To detect SQL problems like that we need to start a clickhouse server to test the SQLs, I want to use Testcontainers to start a clickhouse container and start our exporter based on it. any thought? @googollee @JaredTan95 @hanjm

@Frapschen Looks good, try it. If the CI not support docker, then add a special go tag for local test only.

Copy link
Contributor

github-actions bot commented Feb 5, 2024

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@github-actions github-actions bot added the Stale label Feb 5, 2024
@Frapschen
Copy link
Contributor

fix by #29573

dmitryax pushed a commit that referenced this issue Mar 18, 2024
**Description:** <Describe what has changed.>

as
#29626 (comment)
mentioned, this PR adds an integration test for clickhouse exporter and
it brings those benefits:
- Verify tables are properly created in clickhouse after any breaking
change(filter out bug like
[this](#29573)
before merge)
- Verify if any bugs occur during data insertion like missed data or
insert into an error
column([30210](#30210))
.etc.
- Compatibility testing for `clickhouse-server:23-alpine` and
`clickhouse-server:22-alpine`

close
#29626

---------

Co-authored-by: Tyler Helmuth <12352919+TylerHelmuth@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working exporter/clickhouse Stale
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants