coderabbitai[bot]
commented
1 month ago Walkthrough
The pull request introduces updates to the SigNoz Observability Platform Helm Chart, specifically modifying the Chart.yaml and values.yaml files. The version number of the chart is incremented from 0.53.0 to 0.53.1, while the queryService configuration is enhanced with a new command-line argument to utilize a new logging schema. Additionally, the otelCollector configuration is updated to reflect this new schema, ensuring consistency across the platform's logging capabilities.
Changes
| File | Change Summary |
|---|---|
| charts/signoz/Chart.yaml | Version updated from 0.53.0 to 0.53.1. |
| charts/signoz/values.yaml | queryService.additionalArgs updated to include --use-logs-new-schema=true; new option use_new_schema: true added under otelCollector.clickhouselogsexporter. |
Possibly related PRs
-
507: The changes in this PR also involve updates to the
Chart.yamlandvalues.yamlfiles for the SigNoz Observability Platform, specifically related to versioning and configuration options, making it directly relevant to the main PR's updates.
Suggested reviewers
- YounixM
- srikanthccv
Poem
In the meadow where logs do flow,
A new schema helps us grow.
With a hop and a skip, we update the chart,
Versioning changes, a fresh start!
So let’s cheer for the code, let it gleam,
For the SigNoz team, we’re living the dream! 🐇✨
:scroll: Recent review details
**Configuration used: CodeRabbit UI** **Review profile: CHILL**:inbox_tray: Commits
Files that changed from the base of the PR and between fc4f0f401a070d79536f30ee3991e88503cc36d4 and 483fbf8fd65480bf84614512816a175be544c9dc.:ledger: Files selected for processing (2)
* charts/signoz/Chart.yaml (1 hunks) * charts/signoz/values.yaml (2 hunks):mute: Additional comments not posted (4)
charts/signoz/Chart.yaml (1)
`3-3`: **Version bump looks good.** The increment from 0.53.0 to 0.53.1 is appropriate for the described changes. It follows semantic versioning principles for a minor update. However, I noticed that the PR objectives mention enabling a new logs schema by default and removing a comment about delta preference in query-service. These changes are not reflected in this file. Could you please confirm if these changes are implemented in other files not provided for this review? To verify the implementation of the new logs schema and the removal of the delta preference comment, we can run the following script::white_check_mark: Verification successful
**All changes have been verified and approved.** - Version updated to `0.53.1` in `charts/signoz/Chart.yaml`. - New logs schema enabled in `charts/signoz/values.yaml` with `--use-logs-new-schema=true`. - Delta preference comment successfully removed.:checkered_flag: Scripts executed
The following scripts were executed for the analysis: Script: ```shell #!/bin/bash # Description: Search for new logs schema implementation and delta preference comment removal # Search for new logs schema implementation echo "Searching for new logs schema implementation:" rg --type yaml -i "logs.*schema" # Search for delta preference comment removal echo "Searching for delta preference comment removal:" rg --type yaml -i "delta.*preference" ``` Length of output: 339charts/signoz/values.yaml (3)
Line range hint `1-1981`: **Summary: New logs schema enabled by default** The changes in this file consistently enable the new logs schema by default in both the query service and the OpenTelemetry Collector. This aligns well with the PR objectives and should improve the logging capabilities of SigNoz. Key points to consider: 1. Backward compatibility with existing log data and queries. 2. ClickHouse schema compatibility with the new log format. 3. Potential impact on log ingestion and querying performance. Overall, the changes look good, but thorough testing is recommended to ensure a smooth transition to the new schema. --- `1981-1981`: **LGTM. Verify ClickHouse schema compatibility.** The addition of `use_new_schema: true` to the ClickHouse logs exporter configuration is consistent with the PR objective and the previous change in the query service. This change looks good and should enable the new logs schema in the OpenTelemetry Collector. To ensure smooth operation with the new schema, please verify the ClickHouse schema compatibility: ```shell #!/bin/bash # Description: Check ClickHouse schema compatibility with the new logs schema # Test: Retrieve the current schema for the logs table in ClickHouse echo "Retrieving current ClickHouse schema for logs table:" clickhouse-client --host \${CLICKHOUSE_HOST} --port \${CLICKHOUSE_PORT} --user \${CLICKHOUSE_USER} --password \${CLICKHOUSE_PASSWORD} --query "DESCRIBE TABLE \${CLICKHOUSE_LOG_DATABASE}.logs" # Note: Manually review the output to ensure it matches the expected schema for the new logs format ``` --- `647-648`: **LGTM. Verify backward compatibility.** The addition of `--use-logs-new-schema=true` to the query service arguments aligns with the PR objective of enabling the new logs schema by default. This change looks good, but it's important to ensure that it doesn't break existing functionality or queries on older log data. To verify backward compatibility, you can run the following script:
Features
Summary by CodeRabbit
New Features
Version Update