[exporter/clickhouse] Update default logs table schema #33611
Conversation
| ) ENGINE = %s | ||
| PARTITION BY toYYYYMM(TimestampDate) | ||
| ORDER BY (TimestampDate, TimestampTime) |
There was a problem hiding this comment.
general LGTM. just wondering ORDER BY (TimestampDate, TimestampTime) may not better than ORDER BY ServiceName, SeverityText, toUnixTimestamp(Timestamp), beacause ServiceName and SeverityText( also as well-known logLevel) is the most common filter when query service log, can you write more text explain why remove them? thanks.
There was a problem hiding this comment.
It can be difficult to model a decent table that works for everyone (consider issues like #32215)
The idea behind this change is to simply make time range queries work as best they can. Every log has a timestamp, but not always a proper ServiceName or Severity*. Some OTel data isn't composed very well. SeverityText might not be as good as SeverityNumber, and if we try to add all of these to ORDER BY then it starts to lose its purpose as a primary index.
For those who will be using this exporter in production, the goal is to have them use this as a starting point, and then add the appropriate columns to the ORDER BY depending on their primary column. For example, look at how this blog post chooses to use PodName and Timestamp:
ORDER BY (PodName, Timestamp)What we've found is that a lot of these values are not necessary in the order by, especially ones after the Timestamp* columns (such as TraceId in the current version). Most log queries are going to be in a tight range of time (5, 15, 30, 60 minutes) and these smaller ORDER BY clauses favor this. Again, you can always go a step further by adding your preferred column to the start of it, such as ServiceName, SeverityText, PodName, or some combination of these.
The goal is to have a good default that is easy for others to expand on for their own deployment, while also still giving a good default for those who don't bother to configure one. If you're at the scale where this schema becomes a problem, you would already be using your own schema.
There was a problem hiding this comment.
ServiceName is a required in spec. https://opentelemetry.io/docs/specs/semconv/resource/#service. to say the least, ServiceName is needed in order by columns. see also as #31670
in PodName blog case , i think most common usage query pattern is PodName like xxx%, not PodName= xxx, mostly query a workload log, not the specific pod log. actually, use a serviceName as workload name will be more reasonaly.
There was a problem hiding this comment.
This is reasonable enough since it is a default schema, and if someone needs to change it they're free to do so for their deployment.
As you suggested, I have added ServiceName to the default schema.
Absolutely, but they will need to be evaluated separately (especially metrics) |
…33615) **Description:** *NOTE: Depends on merging of #33611, #33614, and #33648* Logs configuration is unlikely to change further with the new recommendation of [manual schema config](https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/exporter/clickhouseexporter#schema-management). The logs component of the exporter has proven stability, as seen in deployments such as [ClickHouse's Loghouse](https://clickhouse.com/blog/building-a-logging-platform-with-clickhouse-and-saving-millions-over-datadog). With these points in mind, I suggest we upgrade the status to `beta` for logs. Traces and metrics can remain `alpha` for now. **Documentation:** - Updated metadata.yaml - Updated README - Small unrelated change: added/refactored a section that covers tool visualization
**Description:** Previously updated in #33611, I am opening this to start a discussion on further improvements that can be made to the table. Notable changes: - Changed from monthly partitions to daily. With `ttl_only_drop_parts=1`, this will help drop data for TTLs shorter than 1 month (such as when your log retention is only 7 days). - Changed `idx_body` granularity to `8`, which should reduce the index size (especially beneficial for cloud services with separate storage) - Removed `TimestampDate` column - Simplified primary key to only use `TimestampTime`. Performance difference is negligible if not better. Also makes queries easier to write-- with the current version it requires that you provide both `TimestampDate` and `TimestampTime` for optimal sorting performance. - Separated and updated order by. Now it matches the primary key, with the addition of `Timestamp`, so that nanoseconds sorting is preserved by default. Let me know if you have any more suggestions. **Link to tracking Issue:** <Issue number if applicable> **Testing:** <Describe what testing was performed and which tests were added.> **Documentation:**
Description:
Closes #32215
This PR updates the default
otel_logstable schema.The intent of this change is to provide a good starting point for users to create their own schema. This schema works as-is, but should be tuned to fit the final workflow when querying (specifically the indices,
TTL, andORDER BY).There are no breaking changes, this schema is compatible with the previous
INSERTstatement. This does not affect existing users, only new users that havecreate_schemaenabled in the config.Notable changes:
DateandDateTimetypes. NanosecondsTimestampis preserved. This does not affect theINSERTsince these are derived byDEFAULTfromTimestamp.TimestampDatetimeTTLis now based onTimestampTimeTraceFlagsandSeverityNumberhave been reduced to aUInt8, since this is all that is required by the OTel specification.LowCardinalitywhere appropriateORDER BYhas been reduced to onlyTimestampDateandTimestampTime. This offers good performance as-is, but if you have a specific workflow that depends onServiceNamefor example, feel free to add it at as the first column.Link to tracking Issue:
Testing:
Integration tests are disabled, but I fixed them locally to test that this change works as intended.
Unit tests are passing as well.
Documentation:
TimestampTimecolumn