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

fix(docker): use env prefix for boolean in collector config #6199

Merged
merged 1 commit into from
Oct 16, 2024

Conversation

prashant-shahi
Copy link
Member

@prashant-shahi prashant-shahi commented Oct 16, 2024

Summary

Fixes for expected type 'bool', got unconvertible type 'string' errors.

Sample errors:

error decoding 'exporters': error reading configuration for \"clickhousetraces\": decoding failed due to the following error(s):\n\n'docker_multi_node_cluster' expected type 'bool', got unconvertible type 'string', value: 'false'\n'low_cardinal_exception_grouping' expected type 'bool', got unconvertible type 'string', value: 'false'
error decoding 'exporters': error reading configuration for \"clickhouselogsexporter\": decoding failed due to the following error(s):\n\n'docker_multi_node_cluster' expected type 'bool', got unconvertible type 'string', value: 'false'

Important

Fixes boolean type errors in OpenTelemetry collector configuration by prefixing environment variables with env:.

  • Configuration Fix:
    • Prefixes boolean environment variables with env: in otel-collector-config.yaml files to fix type errors.
    • Affects docker_multi_node_cluster and low_cardinal_exception_grouping variables.
  • Files Modified:
    • deploy/docker-swarm/clickhouse-setup/otel-collector-config.yaml
    • deploy/docker/clickhouse-setup/otel-collector-config.yaml
    • pkg/query-service/tests/test-deploy/otel-collector-config.yaml

This description was created by Ellipsis for 0221311. It will automatically update as commits are pushed.

Copy link

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

@github-actions github-actions bot added the bug Something isn't working label Oct 16, 2024
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Looks good to me! Reviewed everything up to 0221311 in 10 seconds

More details
  • Looked at 74 lines of code in 3 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. deploy/docker-swarm/clickhouse-setup/otel-collector-config.yaml:134
  • Draft comment:
    Ensure that the environment variables DOCKER_MULTI_NODE_CLUSTER and LOW_CARDINAL_EXCEPTION_GROUPING are set to valid boolean values ('true' or 'false') to avoid type conversion errors.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR addresses the issue of environment variables being interpreted as strings instead of booleans. The fix involves using the 'env:' prefix to ensure proper type conversion. This change is consistent across multiple files, ensuring uniformity and correctness in configuration handling.
2. deploy/docker-swarm/clickhouse-setup/otel-collector-config.yaml:134
  • Draft comment:
    Ensure that all environment variables use the 'env:' prefix for consistency and correct interpretation.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR correctly changes the environment variable syntax to use the 'env:' prefix, which is necessary for the configuration to interpret them as environment variables.

Workflow ID: wflow_cLQAgjSbIjAT1ICl


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

@prashant-shahi prashant-shahi merged commit a6e4928 into develop Oct 16, 2024
14 of 16 checks passed
@prashant-shahi prashant-shahi deleted the fix/docker-collector-boolean branch October 16, 2024 09:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working testing-deploy
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants