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.

Sign up for GitHub

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

Jump to bottom

Rewrite dashboard creator #2828

Merged
merged 9 commits into from
Oct 23, 2023
Merged

Rewrite dashboard creator #2828

merged 9 commits into from
Oct 23, 2023

Conversation

kwapik
Copy link
Contributor

@kwapik kwapik commented Oct 19, 2023 •
edited
Loading

Checklist
  • Tested in playground or other setup
  • Screenshot (Grafana) from playground added to PR for 15+ minute run
  • Documentation is changed or added

Resolves: #2665

Summary by CodeRabbit

  • Refactor: Updated the structure of the codebase to improve modularity and clarity. This includes changes to function signatures and parameters, allowing for more flexibility and customization.
  • New Feature: Added a function to generate a flat representation of the circuit graph, providing a more intuitive way to visualize the system's structure.
  • New Feature: Introduced changes to generate Grafana dashboards based on specific components, policy names, and data sources. This allows for more tailored and detailed monitoring of system performance.
  • Refactor: Updated the policy compilation process to include the policy name, enhancing the standalone consumption of the policy compiler.
  • New Feature: Added functionality to generate Grafana dashboards and process policies in the command-line interface, providing users with more control and customization options.

@DariaKunoichi @kwapik
Rewrite dashboard creator (#2772)
b774afd 
**Refactor:**
- Removed `creator.libsonnet` and associated functionality across
multiple blueprints.
- Updated function signatures and parameters in Grafana panels and
dashboards for better clarity and consistency.
- Modified datasource parameter usage in Elasticsearch, PostgreSQL, and
RabbitMQ panels to pass the entire datasource object instead of just its
name.
- Adjusted import statements and exported names of modules in
`panel_library.libsonnet`.

**New Feature:**
- Added new functions and made changes to the command-line utility in
`generate.go` and `utils.go`.

**Chore:**
- Made minor changes in `infra_meter_panel_library.libsonnet` and
`compile.go`.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 19, 2023 •
edited
Loading

Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • If you reply to a review comment from CodeRabbit, the bot will automatically respond.
  • To engage with CodeRabbit bot directly around the specific lines of code in the PR, mention @coderabbitai in your review comment
  • Note: Review comments are made on code diffs or files, not on the PR overview.
  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai help to get help.

Note: For conversation with the bot, please use the review comments on code diffs or files.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.json

coderabbitai[bot]
coderabbitai bot reviewed Oct 19, 2023
View reviewed changes
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 9

Commits Files that changed from the base of the PR and between 369ffc7 and b774afd.
Files selected for processing (61)
  • blueprints/auto-scaling/pod-auto-scaler/bundle.libsonnet (2 hunks)
  • blueprints/grafana/dashboard_group.libsonnet (1 hunks)
  • blueprints/grafana/infra_meter_dashboard.libsonnet (1 hunks)
  • blueprints/grafana/infra_meter_panel_library.libsonnet (1 hunks)
  • blueprints/grafana/panel_library.libsonnet (1 hunks)
  • blueprints/grafana/panels/accept_percentage.libsonnet (1 hunks)
  • blueprints/grafana/panels/accepted_vs_rejected_token_rate.libsonnet (1 hunks)
  • blueprints/grafana/panels/average_load_multiplier.libsonnet (1 hunks)
  • blueprints/grafana/panels/avg_preemption_chart.libsonnet (1 hunks)
  • blueprints/grafana/panels/avg_preemption_time_series.libsonnet (1 hunks)
  • blueprints/grafana/panels/elasticsearch/es_static.libsonnet (1 hunks)
  • blueprints/grafana/panels/elasticsearch/es_time_series.libsonnet (2 hunks)
  • blueprints/grafana/panels/grouped/auto_scale.libsonnet (1 hunks)
  • blueprints/grafana/panels/grouped/load_ramp.libsonnet (1 hunks)
  • blueprints/grafana/panels/grouped/load_scheduler.libsonnet (1 hunks)
  • blueprints/grafana/panels/grouped/quota_scheduler.libsonnet (1 hunks)
  • blueprints/grafana/panels/grouped/signals.libsonnet (1 hunks)
  • blueprints/grafana/panels/incoming_token_rate.libsonnet (1 hunks)
  • blueprints/grafana/panels/pgsql/pgsql_bar_gauge.libsonnet (1 hunks)
  • blueprints/grafana/panels/pgsql/pgsql_static.libsonnet (1 hunks)
  • blueprints/grafana/panels/pgsql/pgsql_time_series.libsonnet (1 hunks)
  • blueprints/grafana/panels/query.libsonnet (1 hunks)
  • blueprints/grafana/panels/rabbitmq/rmq_static.libsonnet (1 hunks)
  • blueprints/grafana/panels/rabbitmq/rmq_time_series.libsonnet (1 hunks)
  • blueprints/grafana/panels/rate_limiter.libsonnet (1 hunks)
  • blueprints/grafana/panels/request_in_queue_duration.libsonnet (1 hunks)
  • blueprints/grafana/panels/request_queue_duration_bar.libsonnet (1 hunks)
  • blueprints/grafana/panels/signal_average.libsonnet (2 hunks)
  • blueprints/grafana/panels/signal_frequency.libsonnet (2 hunks)
  • blueprints/grafana/panels/throughput.libsonnet (1 hunks)
  • blueprints/grafana/panels/token_bucket_available_tokens.libsonnet (1 hunks)
  • blueprints/grafana/panels/token_bucket_capacity.libsonnet (1 hunks)
  • blueprints/grafana/panels/token_bucket_fillrate.libsonnet (1 hunks)
  • blueprints/grafana/panels/total_accepted_requests.libsonnet (1 hunks)
  • blueprints/grafana/panels/total_accepted_tokens.libsonnet (1 hunks)
  • blueprints/grafana/panels/total_incoming_tokens.libsonnet (1 hunks)
  • blueprints/grafana/panels/total_rejected_requests.libsonnet (1 hunks)
  • blueprints/grafana/panels/total_rejected_tokens.libsonnet (1 hunks)
  • blueprints/grafana/panels/total_requests.libsonnet (1 hunks)
  • blueprints/grafana/panels/wfq_scheduler_flows.libsonnet (1 hunks)
  • blueprints/grafana/panels/wfq_scheduler_heap_requests.libsonnet (1 hunks)
  • blueprints/grafana/panels/workload_decisions.libsonnet (1 hunks)
  • blueprints/grafana/panels/workload_decisions_accepted.libsonnet (1 hunks)
  • blueprints/grafana/panels/workload_decisions_rejected.libsonnet (1 hunks)
  • blueprints/grafana/panels/workload_latency.libsonnet (1 hunks)
  • blueprints/grafana/summary_dashboard.libsonnet (1 hunks)
  • blueprints/grafana/utils/bar_chart_panel.libsonnet (1 hunks)
  • blueprints/grafana/utils/base_dashboard.libsonnet (1 hunks)
  • blueprints/grafana/utils/unwrap_panels.libsonnet (1 hunks)
  • blueprints/load-ramping/base/bundle.libsonnet (2 hunks)
  • blueprints/load-scheduling/average-latency/bundle.libsonnet (2 hunks)
  • blueprints/load-scheduling/elasticsearch/bundle.libsonnet (2 hunks)
  • blueprints/load-scheduling/java-gc-generic/bundle.libsonnet (2 hunks)
  • blueprints/load-scheduling/java-gc-k8s/bundle.libsonnet (2 hunks)
  • blueprints/load-scheduling/postgresql/bundle.libsonnet (2 hunks)
  • blueprints/load-scheduling/promql/bundle.libsonnet (2 hunks)
  • blueprints/quota-scheduling/base/bundle.libsonnet (2 hunks)
  • blueprints/rate-limiting/base/bundle.libsonnet (2 hunks)
  • cmd/aperturectl/cmd/blueprints/generate.go (8 hunks)
  • cmd/aperturectl/cmd/compile.go (1 hunks)
  • cmd/aperturectl/cmd/utils/utils.go (1 hunks)
Files skipped from review due to trivial changes (11)
  • blueprints/auto-scaling/pod-auto-scaler/bundle.libsonnet
  • blueprints/grafana/infra_meter_panel_library.libsonnet
  • blueprints/grafana/panels/elasticsearch/es_static.libsonnet
  • blueprints/grafana/panels/pgsql/pgsql_static.libsonnet
  • blueprints/load-ramping/base/bundle.libsonnet
  • blueprints/load-scheduling/elasticsearch/bundle.libsonnet
  • blueprints/load-scheduling/postgresql/bundle.libsonnet
  • blueprints/load-scheduling/promql/bundle.libsonnet
  • blueprints/rate-limiting/base/bundle.libsonnet
  • cmd/aperturectl/cmd/compile.go
  • blueprints/grafana/panels/elasticsearch/es_time_series.libsonnet (diff too large)
Additional comments (Suppressed): 122
blueprints/grafana/panels/accept_percentage.libsonnet (3)

  • 4-12: The function signature has been changed from function(cfg) to function(datasourceName, policyName, component, extraFilters={}). Ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify that the component parameter passed to the function has the component_id property as it is being used in line 5.

  • 5-5: The extraFilters parameter is now being used directly in the dictToPrometheusFilter function. Ensure that the extraFilters object passed to the function is in the correct format and contains the necessary keys and values.

  • 9-11: The datasourceName parameter is now being used directly in the timeSeriesPanel function. Ensure that the datasourceName passed to the function is in the correct format and is a valid datasource name.

blueprints/grafana/panels/grouped/load_ramp.libsonnet (2)

  • 6-13: The function signature has been changed from function(cfg) to function(datasourceName, policyName, component, extraFilters={}). Ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify that the extraFilters parameter is always provided as a dictionary, as the function now expects it to be in this format.

  • 8-11: The throughput and signal_average functions are now being called with more parameters. Ensure that these functions have been updated to accept these additional parameters and that they are used correctly within the functions.

blueprints/grafana/dashboard_group.libsonnet (4)

  • 1-3: The import statements have been updated to include infra_meter_dashboard.libsonnet and summary_dashboard.libsonnet instead of creator.libsonnet. This change aligns with the PR summary stating that creator.libsonnet has been removed. Ensure that these new modules provide the necessary functionality that was previously provided by creator.libsonnet.

  • 5-5: The function signature has been updated to include policyFile, componentsList, policyName, datasource, and extraFilters as parameters instead of policyJSON and cfg. This change provides more flexibility and clarity by allowing the caller to provide specific values for these parameters. However, ensure that all calls to this function throughout the codebase have been updated to match the new signature.

  • 6-7: The summary and receivers local variables are now being assigned the return values of summaryDashboard and infraMetersDashboards functions respectively. This is a change from the previous version where mainDashboard and receiverDashboards were assigned the values from the creator function. Ensure that these new functions provide the necessary functionality that was previously provided by the creator function.

  • 9-11: The function now returns a dashboards object that includes the summary dashboard and the receivers dashboards. This is a change from the previous version where mainDashboard, signalsDashboard, and receiverDashboards were returned. Ensure that this new return value is handled correctly in all places where this function is called.

blueprints/grafana/panels/average_load_multiplier.libsonnet (3)

  • 4-5: The function signature has been changed and the cfg object has been replaced with individual parameters. Ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify that the component parameter passed to the function has the component_id field.

  • 5-5: The extra_filters parameter is now a dictionary that can be empty by default. This change improves the flexibility of the function, but make sure that the extra_filters parameter is always passed as a dictionary when calling this function.

  • 7-10: The avgLoadMultiplier function now uses the datasourceName parameter instead of cfg.dashboard.datasource.name. This change improves the reusability of the function, but ensure that the datasourceName parameter is always passed as a string when calling this function.

blueprints/grafana/panels/avg_preemption_chart.libsonnet (1)
  • 4-5: The function signature has been changed from accepting a single cfg object to multiple parameters. This change improves the modularity and reusability of the function. However, ensure that all calls to this function throughout the codebase have been updated to match the new signature.

9:
The component parameter is not used in the function. If it's not needed, consider removing it to avoid confusion.

blueprints/grafana/infra_meter_dashboard.libsonnet (3)

  • 7-30: The function signature has been changed to include policyFile, policyName, datasource, and extraFilters. Ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify that the policyFile passed to the function is in the correct format and contains all the necessary fields. The function now returns a dashboards object that includes the receiver dashboards. Make sure this change doesn't break any existing functionality that expects the old return value.

  • 11-17: The function now checks if the policyJSON object has the spec, resources, and infra_meters fields and if the infra_meters field is not empty. If any of these conditions are not met, it defaults to an empty object. This is a good practice as it prevents potential null pointer exceptions.

  • 19-27: The function now generates a receiverDashboards object by iterating over the infraMeters and receivers fields in the policyJSON object. It uses the unwrapInfraMeter function to generate the panels for each receiver dashboard. Ensure that the unwrapInfraMeter function is correctly implemented and that it returns the expected output.

blueprints/grafana/panels/grouped/signals.libsonnet (2)

  • 4-9: The function signature has been changed from function(cfg) to function(datasourceName, policyName, component, extraFilters={}). Ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify that the extraFilters parameter is always provided as an object, as it is used as a default parameter with an empty object {}. If extraFilters is not provided or is provided as a non-object, it could lead to unexpected behavior.

  • 6-7: The function calls to signal_average and signal_frequency have been updated to pass individual parameters instead of a single cfg object. This change improves the readability and maintainability of the code by making it clear what data is being passed to these functions. However, ensure that the signal_average and signal_frequency functions have been updated to accept these new parameters.

blueprints/grafana/panels/grouped/auto_scale.libsonnet (1)
  • 4-15: The function signature has been changed from function(cfg) to function(datasourceName, policyName, component, extraFilters={}). This change increases the flexibility of the function by allowing the caller to provide specific values for the datasource name, policy name, component, and extra filters. However, ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify that the extraFilters parameter is always provided as an object, as the function now defaults to an empty object if it is not provided.
blueprints/grafana/panels/grouped/load_scheduler.libsonnet (2)

  • 9-22: The function signature has been changed from function(cfg) to function(datasourceName, policyName, component, extraFilters={}). This change provides more flexibility and reusability by allowing the caller to provide specific values for the datasource name, policy name, component, and extra filters. However, ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify that the extraFilters parameter is always provided as an object, even if it's empty, to prevent potential type errors.

  • 10-21: The function calls to quota_scheduler, average_load_multiplier, token_bucket_capacity, token_bucket_fillrate, and token_bucket_available_tokens have been updated to pass the new parameters. This is consistent with the changes made to the function signature. However, ensure that the function signatures of these called functions have also been updated to accept these new parameters.

blueprints/grafana/panels/accepted_vs_rejected_token_rate.libsonnet (4)

  • 6-22: The function signature has been changed from function(cfg) to function(datasourceName, policyName, component, extraFilters={}). This change improves the modularity and reusability of the function by allowing the caller to provide specific values for the datasource name, policy name, component, and extra filters. However, ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, the component parameter is not used in the function, consider removing it if it's not needed.

  • 7-7: The extraFilters parameter is now a dictionary that defaults to an empty dictionary if not provided. This is a good practice as it allows the function to be called with or without this parameter. However, ensure that the extraFilters parameter is always passed as a dictionary when calling this function to avoid type errors.

  • 9-17: The Prometheus queries for 'Accepted Token Rate' and 'Rejected Token Rate' have been updated to use the new datasourceName and stringFilters variables. This change improves the flexibility of the function by allowing the caller to specify the datasource name and filters. Ensure that the datasource name and filters are correctly set when calling this function.

  • 19-19: The timeSeriesPanel function call has been updated to use the new datasourceName and stringFilters variables. This change improves the flexibility of the function by allowing the caller to specify the datasource name and filters. Ensure that the datasource name and filters are correctly set when calling this function.

blueprints/grafana/panel_library.libsonnet (1)
  • 1-13: The new hunk has significantly reduced the number of imported panels. This could potentially impact the functionality of the Grafana dashboards if these panels are still in use elsewhere in the codebase. Please verify that these removed panels are no longer needed or have been replaced with equivalent functionality in the remaining panels. Also, the import paths for the grouped panels are identical, which might be an error. Please verify if this is intentional or if each grouped panel should have a unique import path.
-  AdaptiveLoadScheduler: import './panels/grouped/load_scheduler.libsonnet',
-  AIMDLoadScheduler: import './panels/grouped/load_scheduler.libsonnet',
-  RangeDrivenLoadScheduler: import './panels/grouped/load_scheduler.libsonnet',
-  AIADLoadScheduler: import './panels/grouped/load_scheduler.libsonnet',
+  AdaptiveLoadScheduler: import './panels/grouped/adaptive_load_scheduler.libsonnet',
+  AIMDLoadScheduler: import './panels/grouped/aimd_load_scheduler.libsonnet',
+  RangeDrivenLoadScheduler: import './panels/grouped/range_driven_load_scheduler.libsonnet',
+  AIADLoadScheduler: import './panels/grouped/aiad_load_scheduler.libsonnet',
blueprints/grafana/panels/grouped/quota_scheduler.libsonnet (3)

  • 22-22: The function signature has been changed from accepting a single configuration object cfg to multiple parameters: datasourceName, policyName, component, and extraFilters. This change increases the flexibility of the function, but it also means that all calls to this function throughout the codebase need to be updated to match the new signature. Please verify that this has been done.

  • 24-77: The function calls to generate the panels have been updated to pass the new parameters instead of the configuration object. This change aligns with the modification of the function signature and improves the clarity of the function calls by explicitly specifying the parameters. However, it's important to ensure that the functions being called have also been updated to accept these new parameters. Please verify this.

  • 24-77: The panel positioning logic remains the same, but the parameters passed to the panel generation functions have been updated. This change improves the readability and maintainability of the code by making the function calls more explicit. However, it's important to ensure that the panel positioning still works as expected after these changes. Please verify this.

Overall, the changes in this hunk seem to be focused on improving the flexibility and reusability of the function by allowing the caller to provide specific values for the datasource name, policy name, component, and extra filters. This is a good practice as it makes the function more versatile and easier to use in different contexts.

blueprints/grafana/panels/signal_frequency.libsonnet (2)

  • 6-10: The function signature has been changed to accept individual parameters instead of a configuration object. This change improves the flexibility and reusability of the function. However, ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, the third parameter _ is not used in the function. If this is intentional and meant for future use or to maintain a consistent function signature across multiple functions, it's fine. Otherwise, consider removing it.

  • 15-26: The cfg.dashboard.datasource.name has been replaced with datasourceName in the Prometheus queries. This change is consistent with the updated function signature. The Prometheus queries and the call to timeSeriesPanel function seem to be correctly updated with the new parameters. No issues found here.

blueprints/grafana/panels/avg_preemption_time_series.libsonnet (3)

  • 4-4: The function signature has been changed from accepting a single cfg object to multiple parameters: datasourceName, policyName, component, and extraFilters. This change improves the readability and clarity of the function, but it may require updates to all calls to this function throughout the codebase to match the new signature. Please verify that all such calls have been updated accordingly.

  • 5-5: The extraFilters parameter is now being used directly in the dictToPrometheusFilter function call, and the policy_name is being added to it. This change seems to be aimed at improving the flexibility of the function by allowing the caller to provide specific values for the extra filters. However, it's important to ensure that the policy_name is always provided, either as part of extraFilters or separately, as it's used in the Prometheus query. If policy_name is not provided, it could lead to incorrect results or errors.

  • 9-9: The Prometheus query string has been left unchanged, but the filters placeholder in the string is expected to be replaced with the actual filters. Please ensure that this replacement is happening correctly elsewhere in the code, as it's crucial for the correct execution of the query.

blueprints/grafana/panels/rabbitmq/rmq_time_series.libsonnet (1)
  • 5-54: The changes in this hunk involve passing the entire datasource object to the g.query.prometheus.new() function and the timeSeriesPanel() function instead of just the datasource.name. This change could potentially improve the flexibility of the code by allowing the functions to access other properties of the datasource object if needed. However, it's important to ensure that all calls to these functions throughout the codebase have been updated to match the new function signatures. Also, verify that the datasource object always has the necessary properties and that accessing these properties won't cause any runtime errors.
- g.query.prometheus.new(datasource.name, ...)
+ g.query.prometheus.new(datasource, ...)
- timeSeriesPanel('...', datasource.name, ...)
+ timeSeriesPanel('...', datasource, ...)
blueprints/grafana/panels/token_bucket_capacity.libsonnet (3)

  • 4-11: The function signature has been changed and the cfg parameter has been replaced with datasourceName, policyName, component, and extra_filters. Ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify that the component object passed to the function contains a component_id field, as it is being used in line 5.

  • 5-5: The dictToPrometheusFilter function is being called with a dictionary that includes policy_name and component_id as keys. Ensure that the function can handle these keys correctly and that the component_id key is expected and used appropriately within the function.

  • 7-10: The statPanel function is being called with the parameters 'Token Bucket Capacity', datasourceName, 'avg(token_bucket_capacity_total{%(filters)s})', and stringFilters. Ensure that the statPanel function can handle these parameters correctly and that the datasourceName and stringFilters values are valid and appropriate for the function.

blueprints/grafana/panels/pgsql/pgsql_time_series.libsonnet (2)

  • 13-86: The new code changes the way the datasource parameter is used in the Prometheus queries. Previously, only the name property of the datasource object was used, but now the entire datasource object is passed to the g.query.prometheus.new() function. This change could potentially improve the flexibility of the code by allowing the g.query.prometheus.new() function to access other properties of the datasource object if needed. However, it's important to ensure that the g.query.prometheus.new() function and any other functions that use the datasource parameter are updated to handle the entire datasource object and not just its name property.

  • 92-93: The new code removes the .panel property from the checkpointComparison and commitVsRollback objects when they are returned. This could potentially cause issues if the calling code expects these objects to have a .panel property. Please ensure that all code that uses these objects is updated to handle the new structure.

blueprints/grafana/panels/query.libsonnet (4)

  • 4-12: The function signature has been changed from accepting a single cfg object to multiple parameters: datasourceName, policyName, component, and extraFilters. This change improves the modularity and reusability of the function. However, ensure that all calls to this function throughout the codebase have been updated to match the new signature.

  • 5-5: The extraFilters parameter is now being merged with a dictionary containing policy_name and component_id. This is a good practice as it allows the caller to provide additional filters without having to modify the function itself. However, ensure that the policyName and component.component_id are always provided and valid, as they are now required for the function to work correctly.

  • 6-7: The way signalName and query are being extracted from the component parameter has been changed. Previously, these values were extracted from cfg.component_body.query.promql.out_ports.output.signal_name and cfg.component_body.query.promql.query_string respectively. Now, they are being extracted from component.component.out_ports.output.signal_name and component.component.query_string. Ensure that the component parameter being passed to the function matches this new structure.

  • 9-9: The timeSeriesPanel function is now being called with the new parameters datasourceName, query, and stringFilters. This is a good change as it makes the function call more explicit and easier to understand. However, ensure that the timeSeriesPanel function accepts these parameters in the correct order.

blueprints/grafana/panels/pgsql/pgsql_bar_gauge.libsonnet (2)

  • 8-10: The barGaugePanel function now receives the entire datasource object instead of just its name. Ensure that the function implementation has been updated to handle this change. If the function still expects the datasource name, you might need to pass datasource.name instead of datasource.

  • 13-15: Similar to the previous comment, ensure that the barGaugePanel function can handle the entire datasource object. If not, you might need to pass datasource.name instead of datasource.

blueprints/grafana/panels/token_bucket_fillrate.libsonnet (2)

  • 4-5: The function signature has been changed and now takes datasourceName, policyName, component, and extra_filters as parameters instead of a single cfg object. This change improves the modularity and reusability of the function. However, ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify that component always has a component_id property when this function is called.

  • 7-10: The statPanel function is now called with datasourceName and stringFilters directly, instead of accessing them through the cfg object. This change improves the readability of the code. However, ensure that datasourceName and stringFilters are always defined when this function is called.

blueprints/grafana/panels/rate_limiter.libsonnet (2)

  • 4-11: The function signature has been changed from function(cfg) to function(datasourceName, policyName, component, extraFilters={}). Ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify that the component parameter passed to the function has the component_id field as it is being used in line 5.

  • 7-11: The timeSeriesPanel function is now being called with datasourceName instead of cfg.dashboard.datasource.name. This change allows the caller to specify the datasource name when calling the function, improving its flexibility. However, ensure that the datasource name passed to the function is valid and exists in the system.

blueprints/grafana/panels/request_in_queue_duration.libsonnet (3)

  • 4-4: The function signature has been changed from function(cfg) to function(datasourceName, policyName, component, extraFilters={}). Ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify that the component parameter is always an object with a component_id property.

  • 5-5: The extraFilters parameter is now a dictionary that can be empty by default. This change improves the flexibility of the function, allowing the caller to provide specific filters if needed. However, ensure that the dictToPrometheusFilter function can handle an empty dictionary.

  • 7-11: The timeSeriesPanel function is now called with the datasourceName parameter instead of cfg.dashboard.datasource.name. This change improves the reusability of the function by allowing the caller to provide the datasource name. However, ensure that all calls to this function have been updated to pass the correct datasourceName.

blueprints/grafana/panels/total_incoming_tokens.libsonnet (3)

  • 4-5: The function signature has been changed and now takes datasourceName, policyName, component, and extraFilters as parameters instead of a single cfg object. This change improves the modularity and reusability of the function. However, ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify that the component parameter always has a component_id field.

  • 5-5: The dictToPrometheusFilter function now includes policy_name and component_id in the filters. This change seems to be aimed at providing more specific filtering options. Ensure that the policy_name and component_id are always available and valid when this function is called.

  • 7-11: The statPanel function now takes datasourceName and stringFilters directly as parameters instead of extracting them from a cfg object. This change improves the clarity and readability of the code. However, ensure that the datasourceName and stringFilters are always available and valid when this function is called.

blueprints/grafana/panels/rabbitmq/rmq_static.libsonnet (2)

  • 5-48: The change from passing datasource.name to passing the entire datasource object to the statPanel function is a significant change. This change could potentially break the statPanel function if it is not updated to handle the entire datasource object. Please ensure that the statPanel function and any other functions that use the datasource parameter have been updated to handle the entire datasource object instead of just its name.

  • 5-48: The change from passing datasource.name to passing the entire datasource object to the statPanel function could potentially improve the flexibility and reusability of the function. By passing the entire datasource object, the statPanel function can access any properties or methods of the datasource object that it needs, rather than being limited to just the name. This could make the statPanel function more versatile and adaptable to different use cases.

blueprints/grafana/panels/total_rejected_tokens.libsonnet (3)

  • 4-5: The function signature has been changed and the cfg object has been replaced with individual parameters. Ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify that the component parameter is always an object with a component_id property.

  • 5-5: The extraFilters parameter is now a dictionary that can be empty by default. This change improves the flexibility of the function, but make sure that the dictToPrometheusFilter function can handle an empty dictionary.

  • 7-11: The statPanel function is now called with the new parameters. Ensure that the statPanel function has been updated to accept these new parameters and that it can handle the stringFilters parameter being a string representation of a dictionary.

blueprints/grafana/panels/total_requests.libsonnet (2)

  • 4-5: The function signature has been changed and now takes four parameters instead of one. Ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify that the component parameter is an object with a component_id property.

  • 7-11: The statPanel function is called with the new parameters. Ensure that the statPanel function can handle these new parameters correctly and that it has been updated wherever it is defined.

blueprints/grafana/panels/throughput.libsonnet (3)

  • 4-11: The function signature has been changed from function(cfg) to function(datasourceName, policyName, component, extraFilters={}). Ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify that the component parameter passed to the function has the component_id field as it is being used in line 5.

  • 5-5: The dictToPrometheusFilter function is now being called with extraFilters { policy_name: policyName, component_id: component.component_id }. This is a change from the previous version where it was called with cfg.dashboard.extra_filters { policy_name: cfg.policy.policy_name }. Ensure that the new parameters being passed are correct and that they are providing the expected behavior.

  • 7-10: The timeSeriesPanel function is now being called with 'Throughput - Accept/Reject', datasourceName, 'sum(rate(sampler_counter_total{%(filters)s}[$__rate_interval]))', stringFilters. This is a change from the previous version where it was called with 'Throughput - Accept/Reject', cfg.dashboard.datasource.name, 'sum(rate(sampler_counter_total{%(filters)s}[$__rate_interval]))', stringFilters. Ensure that the new parameters being passed are correct and that they are providing the expected behavior.

blueprints/grafana/panels/wfq_scheduler_flows.libsonnet (1)
  • 4-14: The function signature has been changed from function(cfg) to function(datasourceName, policyName, component, extraFilters={}). Ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify that the component parameter passed to the function has the component_id field as it is being used in line 5.
blueprints/grafana/panels/incoming_token_rate.libsonnet (3)

  • 4-4: The function signature has been changed from function(cfg) to function(datasourceName, policyName, component, extraFilters={}). Ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify that the component parameter is always an object with a component_id property, as it's used in line 5.

  • 5-5: The extraFilters parameter is now being used directly in the dictToPrometheusFilter function, and the policy_name and component_id are being added to it. This is a change from the previous version where cfg.dashboard.extra_filters and cfg.policy.policy_name were used. Ensure that the extraFilters parameter always contains the necessary data for the dictToPrometheusFilter function.

  • 7-11: The timeSeriesPanel function is now being called with datasourceName and stringFilters as arguments, instead of cfg.dashboard.datasource.name and stringFilters. This change seems to be in line with the new function signature and the changes made in line 5. Ensure that the datasourceName parameter always contains the necessary data for the timeSeriesPanel function.

blueprints/grafana/panels/wfq_scheduler_heap_requests.libsonnet (3)

  • 4-13: The function signature has been changed from function(cfg) to function(datasourceName, policyName, component, extraFilters={}). Ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify that the component parameter passed to the function has the component_id attribute as it is being used in line 5.

  • 5-5: The extraFilters parameter is now being used to generate the stringFilters instead of cfg.dashboard.extra_filters. This change allows the caller to provide specific filters when calling the function. However, ensure that the extraFilters passed to the function include all necessary filters for the avg(wfq_requests_total{%(filters)s}) query in line 11.

  • 9-13: The datasourceName parameter is now being used instead of cfg.dashboard.datasource.name to create the wfqSchedulerHeapRequests panel. This change allows the caller to specify the datasource name when calling the function. Ensure that the datasource name passed to the function is valid and exists in the Grafana setup.

blueprints/grafana/panels/workload_decisions.libsonnet (3)

  • 4-5: The function signature has been changed from accepting a single cfg object to multiple parameters. Ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify that the component object passed to the function always contains a component_id field, as it is being used directly in the function.

  • 5-5: The extraFilters parameter is optional and has a default value of an empty dictionary. However, it is directly used in the dictToPrometheusFilter function. Make sure that this function can handle an empty dictionary as input.

  • 7-11: The timeSeriesPanel function is being called with the new datasourceName, stringFilters parameters. Ensure that the timeSeriesPanel function can handle these new parameters correctly.

blueprints/grafana/panels/workload_decisions_rejected.libsonnet (3)

  • 4-11: The function signature has been changed from accepting a single cfg object to accepting multiple parameters: datasourceName, policyName, component, and extraFilters. This change improves the modularity and reusability of the function. However, ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify that the component object passed to the function contains a component_id property, as it is used in line 5.

  • 5-5: The extraFilters object is now being merged with a new object that includes policy_name and component_id. This change allows the function to be more flexible by accepting additional filters. However, ensure that the extraFilters object does not contain keys policy_name or component_id as they would be overwritten.

  • 7-11: The timeSeriesPanel function is now being called with datasourceName and stringFilters as arguments instead of cfg.dashboard.datasource.name and cfg.dashboard.extra_filters. This change improves the clarity and readability of the code. However, ensure that the timeSeriesPanel function has been updated to accept these new arguments.

blueprints/grafana/panels/token_bucket_available_tokens.libsonnet (2)

  • 4-5: The function signature has been changed and now takes datasourceName, policyName, component, and extra_filters as parameters. Ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify that the component object passed to the function contains a component_id field.

  • 7-10: The statPanel function is called with the new parameters. Ensure that the statPanel function can handle these new parameters correctly and that the datasourceName is valid and accessible. Also, verify that the Prometheus query string is correctly formatted with the stringFilters.

blueprints/grafana/utils/base_dashboard.libsonnet (3)

  • 4-4: The function signature has been changed from accepting a configuration object cfg to accepting individual parameters title and refresh_interval. This change could potentially break existing calls to this function. Please ensure that all calls to this function throughout the codebase have been updated to match the new signature.

  • 12-12: The dashboard title is now directly passed as a parameter to the function instead of being accessed from the configuration object. This change improves the readability and maintainability of the code by making it clear what parameters the function depends on.

  • 15-15: The refresh interval for the dashboard is now directly passed as a parameter to the function instead of being accessed from the configuration object. This change improves the readability and maintainability of the code by making it clear what parameters the function depends on.

blueprints/grafana/panels/total_rejected_requests.libsonnet (2)

  • 4-5: The function signature has been changed and the cfg parameter has been replaced with datasourceName, policyName, component, and extraFilters. Ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, the extraFilters parameter has a default value of an empty dictionary. Make sure that this default value is appropriate in all use cases of this function.

  • 7-11: The statPanel function is now being called with the new parameters datasourceName and stringFilters. Ensure that the statPanel function has been updated to accept these new parameters and that it can handle the new stringFilters format correctly.

blueprints/grafana/panels/signal_average.libsonnet (2)

  • 4-8: The function signature has been changed to accept individual parameters instead of a configuration object. This change improves the modularity and reusability of the function. However, ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, the function now accepts an additional parameter _ which is not used in the function. If this parameter is not necessary, consider removing it to avoid confusion.

  • 13-16: The datasourceName is now passed directly to the timeSeriesPanel function instead of being accessed from the configuration object. This change improves the flexibility of the function. However, ensure that the datasourceName passed to the function is valid and exists in the system.

blueprints/quota-scheduling/base/bundle.libsonnet (2)

  • 1-3: The import of creator.libsonnet has been removed. Ensure that this does not affect any dependencies or functionality that relied on it.

  • 7-12: The dashboards field is no longer present in the return value. Make sure that this change does not break any code that expects this field to be present.

blueprints/grafana/panels/request_queue_duration_bar.libsonnet (1)
  • 4-5: The function signature has been changed and the cfg parameter has been replaced with datasourceName, policyName, component, and extraFilters. Ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify that the component object passed to the function always has a component_id property.
blueprints/grafana/panels/total_accepted_requests.libsonnet (2)

  • 4-5: The function signature has been changed and the cfg object has been replaced with individual parameters. Ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify that the component parameter passed to the function has the component_id field as it is being used in the stringFilters local variable.

  • 7-11: The datasourceName is now passed directly to the statPanel function instead of accessing it from the cfg object. This change improves the modularity of the function by reducing its dependency on the structure of the cfg object. However, ensure that the datasourceName is correctly passed in all function calls.

blueprints/grafana/utils/unwrap_panels.libsonnet (1)
  • 1-7: The function signature has been changed and the logic for creating a new configuration object has been removed. Ensure that all calls to this function throughout the codebase have been updated to match the new signature and that the removed logic is handled elsewhere or is no longer necessary. Also, the function now seems to be incomplete as it doesn't return anything. Make sure to return the generatedPanels or the unwrapped panels as per the requirement.
- function(componentName, componentBody, config) {
-   local newConfig =
-     if componentName == 'query'
-     then config { component_body: componentBody }
-     else config,
-   local generatedPanels = panelLibrary[std.toString(componentName)](newConfig),
-   panel:
+ function(datasourceName, policyName, component, extra_filters={}) {
+   local generatedPanels = panelLibrary[component.component_name](datasourceName, policyName, component, extra_filters),
+   return generatedPanels
+ }
blueprints/load-scheduling/java-gc-k8s/bundle.libsonnet (2)

  • 1-3: The import statement for creator has been removed. Ensure that all references to creator have been updated or removed accordingly.

  • 28-35: The dashboards field has been removed from the returned object. This might break any code that relies on this field. Please verify that all such dependencies have been updated or removed.

blueprints/load-scheduling/average-latency/bundle.libsonnet (2)

  • 1-3: The import statement for creator has been removed. Ensure that the functionality provided by creator is no longer needed or has been replaced elsewhere in the codebase.

  • 10-17: The dashboards field has been removed from the return value. This might break any code that relies on this field. Please verify that all such instances have been updated or handled appropriately.

blueprints/grafana/panels/total_accepted_tokens.libsonnet (3)

  • 4-5: The function signature has been changed and now takes four parameters instead of one. Ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, the extraFilters parameter is now optional with a default value of an empty dictionary. This is a good practice as it makes the function more flexible and easier to use.

  • 5-5: The dictToPrometheusFilter function now includes policy_name and component_id in the filters. This change seems to be aimed at providing more specific and granular filtering. However, ensure that the component object passed to the function always has a component_id property.

  • 7-11: The statPanel function is now called with the datasourceName directly instead of accessing it from the cfg object. This change improves the modularity and reusability of the function. However, ensure that the datasourceName passed to the function is always valid.

blueprints/grafana/panels/workload_decisions_accepted.libsonnet (2)

  • 4-4: The function signature has been changed from function(cfg) to function(datasourceName, policyName, component, extraFilters={}). Ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify that the component parameter is always an object with a component_id property.

  • 7-11: The timeSeriesPanel function is now being called with datasourceName and stringFilters as arguments instead of cfg.dashboard.datasource.name and cfg.dashboard.extra_filters. This change improves the flexibility of the function by allowing the caller to provide specific values for the datasource name and filters. However, ensure that the datasourceName and stringFilters values are always provided when calling the function.

blueprints/grafana/utils/bar_chart_panel.libsonnet (1)
  • 7-7: The withDatasource function call has been changed and it no longer includes the 'prometheus' argument. This could potentially break the functionality if the function still expects two arguments. Please verify if the function signature has been updated accordingly in the Grafonnet library.
-    + g.panel.barChart.queryOptions.withDatasource('prometheus', dsName)
+    + g.panel.barChart.queryOptions.withDatasource(dsName)
blueprints/load-scheduling/java-gc-generic/bundle.libsonnet (2)

  • 1-3: The import statement for creator has been removed. Ensure that all references to creator have been replaced or removed in the codebase to avoid any undefined reference errors.

  • 28-35: The dashboards property has been removed from the returned object. This might break any code that relies on this property. Ensure that all such dependencies have been updated to reflect this change.

blueprints/grafana/summary_dashboard.libsonnet (4)

  • 8-21: The function signature has been changed to accept componentsList, policyName, datasource, and extraFilters as parameters. Ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify that the componentsList is always passed as a JSON string, as it is being parsed in line 10. If it's not a JSON string, it will cause a runtime error.

  • 12-17: The code is filtering out null values and flattening the array of panels. This is a good practice as it ensures that the final array of panels doesn't contain any null values which could potentially cause issues downstream.

  • 15-15: The unwrap function is being called with datasource, policyName, c, and extraFilters as arguments. Ensure that the unwrap function has been updated to accept these arguments and that it returns an object with a panel property.

  • 19-19: The g.dashboard.withPanels(panels) function is being called to add the panels to the dashboard. Ensure that this function accepts an array of panels and correctly adds them to the dashboard.

blueprints/grafana/panels/workload_latency.libsonnet (3)

  • 4-5: The function signature has been changed from accepting a single cfg object to multiple parameters. Ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify that the component parameter passed to the function has the component_id field as it is being used in the stringFilters definition.

  • 9-10: The Prometheus query string is using a placeholder %(filters)s for filters, but it's not clear where this placeholder is being replaced with the actual stringFilters. If this is handled elsewhere in the code, ensure that the replacement is still functioning correctly with the changes made in this PR.

  • 5-5: The extraFilters parameter is given a default value of an empty dictionary. This is a good practice as it makes the function more robust to missing arguments and reduces the likelihood of runtime errors.

cmd/aperturectl/cmd/blueprints/generate.go (9)

  • 237-240: The processJsonnetOutput function now takes an additional policyName parameter. Ensure that all calls to this function have been updated to pass this new parameter.

  • 248-251: The processJsonnetOutput function now takes an additional policyName parameter. Ensure that all calls to this function have been updated to pass this new parameter.

  • 269-271: The renderOutput function now takes an additional policyName parameter. Ensure that all calls to this function have been updated to pass this new parameter.

  • 277-307: The renderOutput function now takes an additional policyName parameter. Ensure that all calls to this function have been updated to pass this new parameter.

  • 336-339: The saveYAMLFile function now returns the yamlBytes in addition to the error. This is a good practice as it allows the caller to use the generated YAML bytes for further processing. However, ensure that all calls to this function have been updated to handle this new return value.

  • 353-362: The saveJSONFile function now takes an outputFilePath parameter instead of separate path and filename parameters. This is a good practice as it simplifies the function signature and makes the function more flexible. However, ensure that all calls to this function have been updated to pass the outputFilePath parameter.

  • 420-441: The generateGraphs function now takes a circuit parameter instead of content. This is a good practice as it allows the function to work directly with the Circuit object instead of having to unmarshal the YAML content. However, ensure that all calls to this function have been updated to pass the circuit parameter.

  • 443-468: The processPolicy function now returns a componentsList in addition to the circuit. This is a good practice as it allows the caller to use the components list for further processing. However, ensure that all calls to this function have been updated to handle this new return value.

  • 471-518: The generateDashboards function now takes a policyFile, componentsList, and policyName parameters. This is a good practice as it allows the function to generate dashboards based on the specific policy and components list. However, ensure that all calls to this function have been updated to pass these new parameters.

blueprints/grafana/panels/avg_preemption_chart.libsonnet Show resolved Hide resolved
blueprints/grafana/panels/rate_limiter.libsonnet Outdated Show resolved Hide resolved
blueprints/grafana/panels/wfq_scheduler_flows.libsonnet Show resolved Hide resolved
blueprints/grafana/panels/total_rejected_requests.libsonnet Outdated Show resolved Hide resolved
blueprints/grafana/panels/request_queue_duration_bar.libsonnet Outdated Show resolved Hide resolved
blueprints/grafana/panels/request_queue_duration_bar.libsonnet Show resolved Hide resolved
blueprints/grafana/panels/workload_decisions_accepted.libsonnet Outdated Show resolved Hide resolved
cmd/aperturectl/cmd/utils/utils.go Outdated Show resolved Hide resolved
cmd/aperturectl/cmd/blueprints/generate.go Show resolved Hide resolved
coderabbitai[bot]
coderabbitai bot reviewed Oct 19, 2023
View reviewed changes
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 0

Commits Files that changed from the base of the PR and between b774afd and 49e2467.
Files selected for processing (2)
  • blueprints/grafana/utils/bar_chart_panel.libsonnet (1 hunks)
  • blueprints/grafana/utils/table_panel.libsonnet (1 hunks)
Files skipped from review due to trivial changes (1)
  • blueprints/grafana/utils/table_panel.libsonnet
Additional comments (Suppressed): 1
blueprints/grafana/utils/bar_chart_panel.libsonnet (1)
  • 7-7: The withDatasource function now takes an object with uid and name properties instead of two separate string arguments. This change provides more flexibility in specifying the datasource, but it also requires updates to all calls to this function throughout the codebase. Please verify that all such calls have been updated accordingly.
-    + g.panel.barChart.queryOptions.withDatasource('prometheus', dsName)
+    + g.panel.barChart.queryOptions.withDatasource({ uid: dsName, name: 'prometheus' })

@kwapik kwapik force-pushed the kwapik/rewrite_dashboards_creator branch from 49e2467 to c68e8b9 Compare October 19, 2023 11:56
coderabbitai[bot]
coderabbitai bot reviewed Oct 19, 2023
View reviewed changes
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 0

Commits Files that changed from the base of the PR and between b774afd and c68e8b9.
Files selected for processing (2)
  • blueprints/grafana/utils/bar_chart_panel.libsonnet (1 hunks)
  • blueprints/grafana/utils/table_panel.libsonnet (1 hunks)
Additional comments (Suppressed): 2
blueprints/grafana/utils/bar_chart_panel.libsonnet (1)
  • 3-10: The new hunk has replaced the withDatasource method with withType and withUid methods. This change allows for more granular control over the datasource configuration. However, ensure that the dsName passed to withUid is indeed the UID of the datasource and not just its name. If it's just the name, this could lead to incorrect datasource identification.
-    + g.panel.barChart.datasource.withUid(dsName)
+    + g.panel.barChart.datasource.withUid(datasourceUid)

Also, verify that all calls to this function throughout the codebase have been updated to match the new usage.

blueprints/grafana/utils/table_panel.libsonnet (1)
  • 3-10: The changes in this hunk replace the withDatasource method with withType and withUid methods. This change allows for more granular control over the datasource configuration. However, it's important to ensure that all instances where this function is called have been updated to pass the correct parameters. Also, verify that the dsName parameter now represents the UID of the datasource and not the name.
-    + g.panel.table.queryOptions.withDatasource(dsName)
+    + g.panel.table.datasource.withType('prometheus')
+    + g.panel.table.datasource.withUid(dsName)

coderabbitai[bot]
coderabbitai bot reviewed Oct 20, 2023
View reviewed changes
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 2

Commits Files that changed from the base of the PR and between c68e8b9 and 03df054.
Files selected for processing (5)
  • blueprints/grafana/panel_library.libsonnet (1 hunks)
  • blueprints/grafana/panels/workload_decisions.libsonnet (1 hunks)
  • blueprints/grafana/panels/workload_decisions_accepted.libsonnet (1 hunks)
  • blueprints/grafana/panels/workload_decisions_rejected.libsonnet (1 hunks)
  • cmd/aperturectl/cmd/utils/utils.go (1 hunks)
Additional comments (Suppressed): 10
blueprints/grafana/panel_library.libsonnet (1)
  • 1-10: The new hunk significantly reduces the number of imported panels and grouped panels. It seems like the panels have been refactored and grouped into RateLimiter and PromQL modules. This change simplifies the structure and improves the maintainability of the code. However, please ensure that all the functionalities of the removed panels are covered in the new modules. Also, verify that these changes do not break any dependencies in other parts of the codebase.
blueprints/grafana/panels/workload_decisions_accepted.libsonnet (3)

  • 4-12: The function signature has been changed from accepting a single cfg object to multiple parameters. Ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, the component parameter seems to be a complex object with a component and component_id property. It would be helpful to document the expected structure of this parameter for future developers.

  • 6-6: The dictToPrometheusFilter function is now being called with an additional component_id filter. Ensure that the Prometheus metrics being queried support this new filter and that it doesn't affect the results of the query in an unintended way.

  • 8-12: The timeSeriesPanel function is now being called with the datasourceName directly instead of accessing it from the cfg object. Ensure that the datasourceName is always correctly passed to this function.

blueprints/grafana/panels/workload_decisions.libsonnet (2)

  • 4-4: The function signature has been changed. Ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify that the new parameters datasourceName, policyName, component, and extraFilters are being correctly passed in all function calls.

  • 10-10: The string format placeholder %(filters)s is used in the query string, but it is not clear where this placeholder is being replaced with the actual value. Ensure that the placeholder is correctly replaced with the stringFilters value before the query is executed.

blueprints/grafana/panels/workload_decisions_rejected.libsonnet (4)
  • 4-12: The function signature has been changed from accepting a single cfg object to multiple parameters. Ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, the component parameter seems to be a complex object with properties like component and component_id. It would be better to document the expected structure of this parameter for better maintainability.
- function(cfg) {
+ function(datasourceName, policyName, component, extraFilters={}) {

  • 5-5: The std.get function is used to get the load_scheduler_component_id from the component object, and if it doesn't exist, it defaults to component.component_id. This is a good practice for handling optional properties in objects. However, it's important to ensure that either load_scheduler_component_id or component_id always exists in the component object to avoid potential runtime errors.

  • 6-6: The dictToPrometheusFilter function is now being called with extraFilters and a new object containing policy_name and component_id. This is a good practice as it allows for more flexibility in the filters that can be applied. However, ensure that the dictToPrometheusFilter function can handle the new component_id filter.

  • 8-12: The timeSeriesPanel function is now being called with datasourceName instead of cfg.dashboard.datasource.name. This is a good practice as it reduces the coupling between this function and the structure of the cfg object. However, ensure that all calls to this function have been updated to pass the correct datasourceName.

blueprints/grafana/panels/workload_decisions.libsonnet Show resolved Hide resolved
cmd/aperturectl/cmd/utils/utils.go Outdated Show resolved Hide resolved
coderabbitai[bot]
coderabbitai bot reviewed Oct 20, 2023
View reviewed changes
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 2

Commits Files that changed from the base of the PR and between 03df054 and 4953065.
Files selected for processing (5)
  • blueprints/grafana/actuator_library.libsonnet (1 hunks)
  • blueprints/grafana/panel_library.libsonnet (1 hunks)
  • blueprints/grafana/summary_dashboard.libsonnet (1 hunks)
  • blueprints/grafana/utils/unwrap_panels.libsonnet (1 hunks)
  • cmd/aperturectl/cmd/utils/utils.go (3 hunks)
Additional comments (Suppressed): 10
blueprints/grafana/summary_dashboard.libsonnet (1)
  • 1-26: The new hunk introduces a function that generates a Grafana dashboard given a list of components, a policy name, a datasource, and optional extra filters. It uses several imported libraries and Grafana's JSONNET library to create the dashboard. The function checks if each component in the list is in the panel library or if it's an 'Actuator' and its parent is in the actuator library. If either condition is met, it unwraps the component into a panel. The panels are then flattened into an array and added to the base dashboard.

This code seems to be well-structured and follows good practices. However, it's important to ensure that the unwrap function handles errors properly, especially when the component is not found in the panel library or actuator library. If the unwrap function doesn't handle these cases, it could lead to runtime errors.

Also, the function assumes that the componentsList is a JSON string. If it's not, std.parseJson(componentsList) will throw an error. It would be better to add a check to ensure that componentsList is a valid JSON string before parsing it.

blueprints/grafana/panel_library.libsonnet (3)

  • 1-9: The new hunk significantly reduces the number of imported panels and grouped panels. Ensure that the removed panels are not used elsewhere in the codebase, or that their functionality has been incorporated into the remaining panels. Also, verify that the new RateLimiter and PromQL modules are correctly implemented and used throughout the codebase.

  • 1-9: The new hunk has removed several panel imports and replaced them with RateLimiter and PromQL. If these two modules are intended to replace the functionality of the removed panels, ensure that they are comprehensive and that no functionality is lost in the transition. Also, ensure that the new modules are correctly imported and used in the rest of the codebase.

  • 1-9: The new hunk has significantly simplified the panel library by reducing the number of imported panels. This could improve maintainability and readability, but it's important to ensure that the removed panels' functionality is adequately covered by the new RateLimiter and PromQL modules. Also, verify that the new modules are correctly used in the rest of the codebase.

blueprints/grafana/utils/unwrap_panels.libsonnet (1)
  • 1-13: The function signature has been changed and now includes datasourceName, policyName, component, isActuator, and extraFilters. Ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, the function now imports and uses actuatorLibrary in addition to panelLibrary. Make sure that the actuatorLibrary module is correctly implemented and tested. The function now generates panels based on the isActuator flag, which changes the behavior of the function. Verify that this change is intended and correctly implemented. The extraFilters parameter is a new addition and it's used in both actuatorLibrary and panelLibrary calls. Ensure that these libraries can handle this parameter correctly.
cmd/aperturectl/cmd/utils/utils.go (5)

  • 4-8: The import of the encoding/json package is new. Ensure that it is used in the new code and remove it if it's not necessary.

  • 27-31: The import of the github.com/fluxninja/aperture/v2/pkg/utils package is new. Ensure that it is used in the new code and remove it if it's not necessary.

  • 133-155: The new function GetFlatComponentsList returns a flat representation of the circuit graph. It uses the ExpandTreeWithParentInfo function to get a list of components with parent information, then trims the parent name to only include the top-level parent. It then marshals this list into a JSON string. Ensure that the error handling is correct and that the function behaves as expected.

  • 157-162: The componentOutput struct is used to represent a component in the circuit graph. It includes the component ID, name, parent name, and the component itself. Ensure that the struct fields are correctly used in the new code.

  • 164-185: The ExpandTreeWithParentInfo function is a recursive function that expands a tree of components into a flat list, including parent information. It creates a new parent name for each node by appending the node's name to the current parent name. If a node has children, it recurses into them; if not, it adds the node to the list of components. Ensure that the recursion is correctly implemented and that the function behaves as expected.

blueprints/grafana/actuator_library.libsonnet Show resolved Hide resolved
blueprints/grafana/utils/unwrap_panels.libsonnet Show resolved Hide resolved
coderabbitai[bot]
coderabbitai bot reviewed Oct 20, 2023
View reviewed changes
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 5

Commits Files that changed from the base of the PR and between 4953065 and 1d916d7.
Files selected for processing (22)
  • blueprints/grafana/panels/accept_percentage.libsonnet (1 hunks)
  • blueprints/grafana/panels/average_load_multiplier.libsonnet (1 hunks)
  • blueprints/grafana/panels/incoming_token_rate.libsonnet (1 hunks)
  • blueprints/grafana/panels/query.libsonnet (1 hunks)
  • blueprints/grafana/panels/rate_limiter.libsonnet (1 hunks)
  • blueprints/grafana/panels/request_in_queue_duration.libsonnet (1 hunks)
  • blueprints/grafana/panels/request_queue_duration_bar.libsonnet (1 hunks)
  • blueprints/grafana/panels/throughput.libsonnet (1 hunks)
  • blueprints/grafana/panels/token_bucket_available_tokens.libsonnet (1 hunks)
  • blueprints/grafana/panels/token_bucket_capacity.libsonnet (1 hunks)
  • blueprints/grafana/panels/token_bucket_fillrate.libsonnet (1 hunks)
  • blueprints/grafana/panels/total_accepted_requests.libsonnet (1 hunks)
  • blueprints/grafana/panels/total_accepted_tokens.libsonnet (1 hunks)
  • blueprints/grafana/panels/total_incoming_tokens.libsonnet (1 hunks)
  • blueprints/grafana/panels/total_rejected_requests.libsonnet (1 hunks)
  • blueprints/grafana/panels/total_rejected_tokens.libsonnet (1 hunks)
  • blueprints/grafana/panels/total_requests.libsonnet (1 hunks)
  • blueprints/grafana/panels/wfq_scheduler_flows.libsonnet (1 hunks)
  • blueprints/grafana/panels/wfq_scheduler_heap_requests.libsonnet (1 hunks)
  • blueprints/grafana/panels/workload_latency.libsonnet (1 hunks)
  • pkg/policies/controlplane/policy.go (1 hunks)
  • pkg/policies/controlplane/validator.go (1 hunks)
Additional comments (Suppressed): 56
blueprints/grafana/panels/total_accepted_requests.libsonnet (2)

  • 4-6: The function signature has been changed to accept datasourceName, policyName, component, and extraFilters as parameters instead of a single cfg object. This change increases the function's flexibility and readability. However, ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify that the component parameter always has either component or component_id as a property. If not, this could lead to runtime errors.

  • 8-12: The statPanel function is now directly using the datasourceName and stringFilters parameters. This change simplifies the function call and makes the code more readable. However, ensure that the datasourceName and stringFilters are always provided when this function is called to avoid runtime errors.

blueprints/grafana/panels/token_bucket_available_tokens.libsonnet (2)

  • 4-12: The function signature has been changed to accept datasourceName, policyName, component, and extra_filters as parameters instead of a single cfg object. This change increases the function's flexibility and readability. However, ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify that the component parameter always contains either component or component_id as keys. If not, this could lead to runtime errors.

  • 6-6: The dictToPrometheusFilter function is now being called with an additional component_id filter. This change seems to be in line with the PR's goal of providing more flexibility in specifying filters for the Prometheus query. However, ensure that the Prometheus query can handle this additional filter without any issues.

blueprints/grafana/panels/accept_percentage.libsonnet (1)
  • 4-13: The function signature has been changed to accept individual parameters instead of a single configuration object. This change improves readability and makes the function more flexible. However, ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify that the component parameter passed to the function always has either component or component_id as a property, as the function now uses std.get to retrieve the component_id from the component object. If neither of these properties is present, it could lead to unexpected behavior.
blueprints/grafana/panels/query.libsonnet (3)

  • 4-13: The function signature has been changed to accept individual parameters instead of a single configuration object. This change improves readability and makes it easier to understand what inputs the function requires. However, ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify that the component object passed to the function always contains the component and out_ports.output.signal_name properties, and that the component property either contains a load_scheduler_component_id or component_id property.

  • 6-6: The dictToPrometheusFilter function now includes policy_name and component_id in the filters. This change allows for more specific filtering in the Prometheus query. However, ensure that the policyName and componentID values are always available and valid when this function is called.

  • 10-10: The timeSeriesPanel function now accepts the datasourceName directly instead of extracting it from a configuration object. This change simplifies the function call and makes the code more readable. However, ensure that the datasourceName is always a valid and available value when this function is called.

blueprints/grafana/panels/total_rejected_tokens.libsonnet (4)

  • 4-4: The function signature has been changed to accept individual parameters instead of a single configuration object. Ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify that the extraFilters parameter is always passed as a dictionary, as the function assumes it to be a dictionary.

  • 5-5: The std.get function is used to get the component_id from the component object. However, it seems like the function is trying to get the component_id from two different keys: load_scheduler_component_id and component_id. This could lead to unexpected behavior if the component object doesn't have the expected structure. Please verify the structure of the component object and adjust the code accordingly.

  • 6-6: The dictToPrometheusFilter function is called with a dictionary that includes policy_name and component_id. Ensure that the dictToPrometheusFilter function can handle these keys correctly.

  • 8-12: The statPanel function is called with a new set of parameters. Ensure that the statPanel function has been updated to handle these parameters correctly.

blueprints/grafana/panels/request_queue_duration_bar.libsonnet (4)

  • 4-12: The function signature has been changed to accept individual parameters instead of a single configuration object. This change improves readability and makes it easier to understand what parameters are required. However, ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, the component parameter seems to be an object with a component and component_id property. Make sure that the objects passed to this function always have these properties.

  • 5-5: The std.get function is used to get the load_scheduler_component_id property from the component object, and if it doesn't exist, it defaults to component.component_id. This is a good practice as it prevents potential undefined errors. However, ensure that either load_scheduler_component_id or component_id is always present in the component object.

  • 6-6: The dictToPrometheusFilter function is now being called with additional filters (policy_name and component_id). This change allows for more specific Prometheus queries. Ensure that the dictToPrometheusFilter function can handle these additional filters and that they are correctly used in the Prometheus queries.

  • 8-12: The barChartPanel function is now being called with the new datasourceName and stringFilters parameters. Ensure that the barChartPanel function has been updated to handle these new parameters.

blueprints/grafana/panels/token_bucket_capacity.libsonnet (3)

  • 4-12: The function signature has been changed to accept datasourceName, policyName, component, and extra_filters as parameters instead of a single cfg object. This change increases the function's flexibility but also requires updates to all function calls throughout the codebase. The componentID is now retrieved from the component object, which adds an additional layer of flexibility. The Prometheus query filter now includes policy_name and component_id, which should provide more precise data filtering. Ensure that all calls to this function have been updated to match the new signature and that the component objects passed to this function have the necessary properties.

  • 6-6: The dictToPrometheusFilter function is used to convert the extra_filters dictionary and the new policy_name and component_id entries into a Prometheus filter string. Ensure that the dictToPrometheusFilter function can handle all possible types and values of extra_filters, policy_name, and component_id that might be passed to it.

  • 8-11: The statPanel function is called with the new datasourceName and stringFilters parameters. Ensure that the statPanel function can handle these new parameters and that the Prometheus query string is correctly formatted with the new stringFilters.

blueprints/grafana/panels/incoming_token_rate.libsonnet (4)

  • 4-4: The function signature has been changed to accept individual parameters instead of a single configuration object. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.

  • 5-5: The componentID is now being fetched from the component object with a default value of component.component_id. This change assumes that the component object will always have either component or component_id as a property. If this is not the case, it could lead to unexpected behavior.

  • 6-6: The dictToPrometheusFilter function now takes extraFilters with added policy_name and component_id. Ensure that the dictToPrometheusFilter function can handle these new parameters and that they are correctly used in the Prometheus query.

  • 8-12: The timeSeriesPanel function now takes datasourceName directly instead of fetching it from cfg.dashboard.datasource.name. This change simplifies the function call and makes the code more readable.

blueprints/grafana/panels/total_rejected_requests.libsonnet (3)

  • 4-12: The function signature has been changed to accept datasourceName, policyName, component, and extraFilters as parameters instead of a single cfg object. This change increases the flexibility of the function but also requires all calls to this function to be updated. Please verify that all calls to this function have been updated to match the new signature. Also, the component parameter seems to be an object with a component and component_id property. Please ensure that the objects passed to this function always have these properties. If not, consider adding error handling to manage cases where these properties are not present.

  • 6-6: The extraFilters parameter is combined with policyName and componentID to create stringFilters. This is a good use of the extraFilters parameter to allow additional filters to be passed to the function. However, ensure that the extraFilters parameter is always an object to prevent errors when it is combined with policyName and componentID.

  • 8-12: The statPanel function is called with the new datasourceName and stringFilters parameters. This is a good change as it allows the data source and filters to be specified more flexibly. However, ensure that the statPanel function has been updated to accept these new parameters.

blueprints/grafana/panels/total_requests.libsonnet (1)
  • 4-12: The function signature has been changed to accept datasourceName, policyName, component, and extraFilters as parameters instead of a single cfg object. This change increases the flexibility of the function but also requires updates to all function calls throughout the codebase. The componentID is now retrieved from the component object, which adds a new dependency. Ensure that the component object always contains either a component or component_id property. The stringFilters now includes policyName and componentID, which might affect the Prometheus query results. Verify that these changes are intended and correctly implemented.
blueprints/grafana/panels/throughput.libsonnet (1)
  • 4-12: The function signature has been changed to accept datasourceName, policyName, component, and extraFilters as parameters instead of a single cfg object. This change increases the function's flexibility and readability. However, ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify that the component parameter always has a component or component_id property.
- function(cfg) {
-   local stringFilters = utils.dictToPrometheusFilter(cfg.dashboard.extra_filters { policy_name: cfg.policy.policy_name }),
-   local throughput = timeSeriesPanel('Throughput - Accept/Reject',
-                                      cfg.dashboard.datasource.name,
-                                      'sum(rate(sampler_counter_total{%(filters)s}[$__rate_interval]))',
-                                      stringFilters),
- }
+ function(datasourceName, policyName, component, extraFilters={}) {
+   local componentID = std.get(component.component, 'load_scheduler_component_id', default=component.component_id),
+   local stringFilters = utils.dictToPrometheusFilter(extraFilters { policy_name: policyName, component_id: componentID }),
+   local throughput = timeSeriesPanel('Throughput - Accept/Reject',
+                                      datasourceName,
+                                      'sum(rate(sampler_counter_total{%(filters)s}[$__rate_interval]))',
+                                      stringFilters),
+ }
blueprints/grafana/panels/token_bucket_fillrate.libsonnet (2)

  • 4-12: The function signature has been changed to accept datasourceName, policyName, component, and extra_filters as parameters instead of a single cfg object. This change improves the readability and clarity of the function. However, ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify that the component parameter passed to the function always has either component or component_id as a property. If not, this could lead to unexpected behavior or errors.

  • 6-6: The dictToPrometheusFilter function now includes policy_name and component_id in the filters. This change allows for more specific filtering in the Prometheus query, which can improve the accuracy of the data displayed on the Grafana dashboard. However, ensure that the Prometheus data source includes these labels in its data, or the query may not return the expected results.

pkg/policies/controlplane/validator.go (1)
  • 94-94: The function CompilePolicy now takes an additional parameter name. Ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify that the name parameter is being used correctly within the CompilePolicy function.
-    circuit, err := CompilePolicy(policy, registry)
+    circuit, err := CompilePolicy(policy, name, registry)
blueprints/grafana/panels/wfq_scheduler_flows.libsonnet (3)

  • 4-15: The function signature has been changed to accept individual parameters instead of a single configuration object. This change improves readability and makes the function more flexible. However, ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify that the component parameter passed to the function always has either component or component_id as a property. If not, this could lead to unexpected behavior.

  • 6-6: The dictToPrometheusFilter function is now being called with additional filters policy_name and component_id. This is a good practice as it allows more specific filtering of the Prometheus data. However, ensure that the Prometheus data being queried supports these additional filters.

  • 11-15: The barGaugePanel function is now being called with the datasourceName directly instead of getting it from the cfg object. This is a good practice as it makes the function more flexible and reduces dependencies on the structure of the cfg object. However, ensure that the datasourceName passed to the function is always valid and exists in the system.

blueprints/grafana/panels/total_accepted_tokens.libsonnet (2)

  • 4-6: The function signature has been changed to accept individual parameters instead of a single configuration object. This change improves readability and makes the function more flexible. However, ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify that the component parameter always has a component or component_id field.

  • 8-12: The statPanel function is now called with the datasourceName and stringFilters directly passed as arguments. This change simplifies the function call and makes the code more readable. However, ensure that the statPanel function has been updated to accept these parameters directly.

blueprints/grafana/panels/total_incoming_tokens.libsonnet (2)

  • 4-12: The function signature has been changed to accept datasourceName, policyName, component, and extraFilters as parameters instead of a single cfg object. This change improves the readability and clarity of the function. However, ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify that the component parameter always has either component or component_id as a property.

  • 6-6: The dictToPrometheusFilter function is now being called with extraFilters and a dictionary containing policy_name and component_id. This change allows for more flexibility in specifying additional filters for the Prometheus query. Ensure that the dictToPrometheusFilter function can handle the new dictionary structure.

pkg/policies/controlplane/policy.go (1)
  • 83-88: The function signature of CompilePolicy has been changed to include an additional policyName parameter. This change allows the function to be used for standalone consumption of the policy compiler with a specific policy name. Ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify that the policyName parameter is being used correctly within the function.
blueprints/grafana/panels/workload_latency.libsonnet (4)

  • 4-12: The function signature has been changed to accept datasourceName, policyName, component, and extraFilters as parameters instead of a single cfg object. This change increases the function's flexibility and readability. However, ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify that the component parameter always has either component or component_id as a property. If not, this could lead to runtime errors.

  • 5-5: The std.get function is used to get the load_scheduler_component_id from the component object, and if it doesn't exist, it defaults to component.component_id. This is a good practice for handling optional object properties. However, ensure that at least one of these properties is always present in the component object to avoid undefined values.

  • 6-6: The dictToPrometheusFilter function is now called with extraFilters merged with a new object containing policy_name and component_id. This is a good practice as it allows the function to handle additional filters dynamically. However, ensure that extraFilters is always an object to avoid runtime errors.

  • 8-12: The timeSeriesPanel function is now called with datasourceName and stringFilters as parameters. This change increases the function's flexibility and readability. However, ensure that all calls to this function throughout the codebase have been updated to match the new usage.

blueprints/grafana/panels/request_in_queue_duration.libsonnet (4)

  • 4-4: The function signature has been changed to accept individual parameters instead of a single configuration object. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.

  • 5-5: The componentID is now being fetched from the component object with a default value of component.component_id. This change assumes that the component object will always have either component or component_id as a property. If this is not the case, it could lead to unexpected behavior.

  • 6-6: The stringFilters now includes policy_name and component_id in addition to the extraFilters. This is a good improvement as it allows more flexibility in specifying the data source and additional filters for the Prometheus query.

  • 8-12: The timeSeriesPanel function is now being called with the new datasourceName and stringFilters parameters. Ensure that the timeSeriesPanel function has been updated to handle these new parameters correctly.

blueprints/grafana/panels/average_load_multiplier.libsonnet (2)

  • 4-12: The function signature has been changed to accept datasourceName, policyName, component, and extra_filters as parameters instead of a single cfg object. This change increases the function's flexibility and clarity. However, ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify that the component parameter passed to the function always contains either component or component_id as a key. If not, it could lead to unexpected behavior due to the use of std.get in line 5.

  • 6-6: The dictToPrometheusFilter function now includes policy_name and component_id in the extra_filters dictionary. This change allows for more specific filtering in the Prometheus query. However, ensure that the policy_name and component_id are always available and correctly set when this function is called, as they are now critical for the Prometheus query.

blueprints/grafana/panels/wfq_scheduler_heap_requests.libsonnet (4)

  • 4-4: The function signature has been changed. Ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify that the new parameters datasourceName, policyName, component, and extraFilters are being correctly passed wherever this function is called.

  • 5-5: The std.get function is used to get the load_scheduler_component_id from the component object. If it doesn't exist, it defaults to component.component_id. Ensure that the component object passed to the function always contains either load_scheduler_component_id or component_id to avoid potential issues.

  • 6-6: The dictToPrometheusFilter function is now being called with extraFilters along with policy_name and component_id. Ensure that the extraFilters parameter is always a dictionary and that the dictToPrometheusFilter function can handle these additional filters.

  • 10-14: The barGaugePanel function is now being called with datasourceName instead of cfg.dashboard.datasource.name and stringFilters instead of cfg.dashboard.extra_filters. Ensure that these changes do not affect the functionality of the barGaugePanel function and that the datasourceName and stringFilters parameters are correctly passed.

blueprints/grafana/panels/rate_limiter.libsonnet (3)

  • 4-12: The function signature has been changed to accept individual parameters instead of a single configuration object. This change improves readability and makes the function more flexible. However, ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify that the component parameter passed to the function always contains either component or component_id as a key. If not, this could lead to a runtime error when trying to access component.component or component.component_id.

  • 6-6: The dictToPrometheusFilter function now includes policyName and componentID in the filters. This change allows for more specific filtering in the Prometheus query. However, ensure that the Prometheus metrics being queried include these labels (policy_name and component_id), otherwise, the query might return no data.

  • 8-12: The timeSeriesPanel function now accepts datasourceName directly instead of accessing it from a configuration object. This change improves readability and makes the function more flexible. However, ensure that the datasourceName passed to the function is always valid and corresponds to a configured data source in Grafana, otherwise, the panel might fail to load data.

blueprints/grafana/panels/accept_percentage.libsonnet Show resolved Hide resolved
blueprints/grafana/panels/total_rejected_requests.libsonnet Show resolved Hide resolved
blueprints/grafana/panels/total_requests.libsonnet Show resolved Hide resolved
blueprints/grafana/panels/wfq_scheduler_flows.libsonnet Show resolved Hide resolved
blueprints/grafana/panels/total_incoming_tokens.libsonnet Show resolved Hide resolved
@kwapik kwapik marked this pull request as ready for review October 20, 2023 13:00
@kwapik kwapik requested review from a team as code owners October 20, 2023 13:00
coderabbitai[bot]
coderabbitai bot reviewed Oct 20, 2023
View reviewed changes
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 1

Commits Files that changed from the base of the PR and between 1d916d7 and 80b0e91.
Files selected for processing (1)
  • cmd/aperturectl/cmd/blueprints/generate.go (8 hunks)
Additional comments (Suppressed): 10
cmd/aperturectl/cmd/blueprints/generate.go (10)

  • 163-173: The new code is extracting the policy and policy_name from the values map. Ensure that the values map always contains these keys, otherwise, the code will return an error. If these keys are optional, consider providing default values or handling their absence gracefully.

  • 238-240: The processJsonnetOutput function now accepts an additional parameter policyName. Ensure that all calls to this function have been updated to pass this new parameter.

  • 248-251: The processJsonnetOutput function now accepts an additional parameter policyName. Ensure that all calls to this function have been updated to pass this new parameter.

  • 269-271: The renderOutput function now accepts an additional parameter policyName. Ensure that all calls to this function have been updated to pass this new parameter.

  • 277-301: The renderOutput function has been significantly refactored. It now includes calls to processPolicy, generateDashboards, and generateGraphs within the condition that checks if the file name ends with -cr.yaml. This change suggests that these functions are now only called for files with this specific extension. Ensure that this change does not affect other parts of the codebase that rely on these functions being called for files with different extensions.

  • 336-339: The saveYAMLFile function now returns the yamlBytes in addition to the error. Ensure that all calls to this function have been updated to handle this new return value.

  • 353-362: The saveJSONFile function signature has been simplified to only require the outputFilePath and content parameters. Ensure that all calls to this function have been updated to match the new signature.

  • 420-441: The generateGraphs function now accepts a circuit parameter of type *circuitfactory.Circuit instead of the content parameter of type []byte. Ensure that all calls to this function have been updated to pass a circuit object instead of a byte slice.

  • 443-468: The processPolicy function has been added. This function unmarshals the policy content, compiles the policy, and gets a flat list of components. It returns a circuit, a componentsList, and an error. Ensure that this function is used correctly in the codebase.

  • 471-518: The generateDashboards function has been added. This function generates Grafana dashboards based on the policy file and components list. It saves the dashboards as JSON files in the dashboards directory. Ensure that this function is used correctly in the codebase.

cmd/aperturectl/cmd/blueprints/generate.go Show resolved Hide resolved
@kwapik
Copy link
Contributor Author

kwapik commented Oct 20, 2023

image

@kwapik kwapik merged commit 60288a7 into main Oct 23, 2023
9 checks passed
@kwapik kwapik deleted the kwapik/rewrite_dashboards_creator branch October 23, 2023 12:36
tanveergill pushed a commit that referenced this pull request Oct 24, 2023
@kwapik @DariaKunoichi @tanveergill
Rewrite dashboard creator (#2828)
357a222 
Co-authored-by: Daria Bialobrzeska <[email protected]>
Co-authored-by: Daria Danilenko <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

@hdkshingala hdkshingala hdkshingala approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add component_id label to filter in dashboard creator
3 participants
@kwapik @hdkshingala @DariaKunoichi