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

chore: skip showing metrics with dot in name #6096

Merged
merged 3 commits into from
Oct 10, 2024
Merged

Conversation

srikanthccv
Copy link
Member

@srikanthccv srikanthccv commented Sep 30, 2024

Summary

Part of #5975

Fixes #6067

We will write to the storage but don't want to show the metrics with dots in the suggestions till we ensure everything works. The existing data write path is not distributed, i.e. what worked earlier would continue to work

It could be done at the query itself, but the number of metrics names is not going to be much and importantly I want to filp the flag and allow dots in upcoming so I don't want to change query back later.


Important

Skip metrics with dots in their names from suggestions by adding a skipDotNames parameter to GetMetricAggregateAttributes.

  • Behavior:
    • Skip metrics with dots in their names in suggestions by adding skipDotNames parameter to GetMetricAggregateAttributes in reader.go.
    • Update autocompleteAggregateAttributes in http_handler.go to pass true for skipDotNames.
  • Interfaces:
    • Add skipDotNames parameter to GetMetricAggregateAttributes in interface.go.

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

@github-actions github-actions bot added the chore label Sep 30, 2024
Base automatically changed from bump-prom to develop October 10, 2024 08:40
@srikanthccv srikanthccv marked this pull request as ready for review October 10, 2024 09:02
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 63b2192 in 27 seconds

More details
  • Looked at 56 lines of code in 3 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. pkg/query-service/interfaces/interface.go:55
  • Draft comment:
    The addition of the skipDotNames parameter to GetMetricAggregateAttributes aligns with the PR's intent to skip metrics with dots in their names. Ensure that all implementations of this interface are updated accordingly.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR introduces a new parameter skipDotNames to the GetMetricAggregateAttributes function in the Reader interface. This change is consistent with the PR's intent to skip metrics with dots in their names. The change is correctly reflected in the interface definition.
2. pkg/query-service/interfaces/interface.go:55
  • Draft comment:
    Avoid adding non-ClickHouse related functions to the ClickHouseReader interface. Use the DAO in the telemetry instance to access such information instead.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable:
    The function GetMetricAggregateAttributes seems to be related to metrics, which could be part of ClickHouse's functionality. The comment does not provide strong evidence that this function is non-ClickHouse related. Without more context, it's difficult to determine if the comment is applicable.
    I might be missing the specific context of what constitutes a non-ClickHouse related function. The function name alone might not be enough to determine its relevance to ClickHouse.
    Given the lack of specific evidence in the comment and the function name suggesting a metric-related operation, it's reasonable to assume the comment might not be applicable.
    Delete the comment as it lacks strong evidence of being relevant to the change made in the diff.

Workflow ID: wflow_6LIan5uHFsyTtueh


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

@srikanthccv srikanthccv merged commit 155a2ea into develop Oct 10, 2024
15 checks passed
@srikanthccv srikanthccv deleted the skip-dot-names branch October 10, 2024 11:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

update query service to hide (from querying/ui) the dot-supported metrics/labels till the data is backfilled.
3 participants