Fix unregisters

[tanveergill]

Description of change

Checklist

• Tested in playground or other setup
• Screenshot (Grafana) from playground added to PR for 15+ minute run
• Documentation is changed or added
• Tests and/or benchmarks are included
• Breaking changes

Summary by CodeRabbit

• Refactor

  • Updated log messages for clearer understanding of system status.
  • Enhanced error handling and warnings for metric operations.

• Bug Fixes

  • Fixed issues with the removal of metrics to ensure proper cleanup during system operations.

[coderabbitai]

Walkthrough

The changes across various files seem to be part of a larger refactoring effort focused on renaming and improving error handling and logging within a system's flow control and metrics processing components. The updates include renaming methods and log messages for clarity, as well as restructuring how metrics are removed and how related errors are handled.

Changes

File Path	Change Summary
.../metricsprocessor/processor.go .../flowcontrol/actuators/sampler/sampler.go	Updated log messages and restructured metric deletion and error handling.
.../flowcontrol/actuators/concurrency-limiter/concurrency-limiter.go .../flowcontrol/actuators/rate-limiter/rate-limiter.go	Modified error handling for metric removal and adjusted related logic.
.../flowcontrol/actuators/concurrency-scheduler/concurrency-scheduler.go	Renamed a method from UnregisterScheduler to UnregisterConcurrencyScheduler.

Poem

🐇
In the warren of code, we hop and refactor,
Renaming functions, with clarity we capture.
Errors now caught, and logs doth enhance,
A merry dance of code, in the binary expanse.
🌟

---

Tips

Chat

There are 3 ways to chat with CodeRabbit's AI:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>.
  • Generate unit-tests for this file.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit tests for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai generate interesting stats about this repository from git and render them as a table.
  • @coderabbitai show all the console.log statements in this repository.
  • @coderabbitai read src/utils.ts and generate unit tests.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid.
  • @coderabbitai read the files in the src/scheduler package and generate README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

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 resolve resolve all the CodeRabbit review comments.
• @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

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.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

[coderabbitai]

Review Status

Actionable comments generated: 1

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between e506d1b and 7a838ad.

Files selected for processing (5)

• pkg/otelcollector/metricsprocessor/processor.go (1 hunks)
• pkg/policies/flowcontrol/actuators/concurrency-limiter/concurrency-limiter.go (2 hunks)
• pkg/policies/flowcontrol/actuators/concurrency-scheduler/concurrency-scheduler.go (1 hunks)
• pkg/policies/flowcontrol/actuators/rate-limiter/rate-limiter.go (2 hunks)
• pkg/policies/flowcontrol/actuators/sampler/sampler.go (2 hunks)

Additional comments: 6

pkg/policies/flowcontrol/actuators/sampler/sampler.go (1)

• 268-271: The logic for deleting the sampler counter from the metric vector and logging a warning if the deletion fails is correctly implemented.

pkg/policies/flowcontrol/actuators/concurrency-limiter/concurrency-limiter.go (1)

• 272-275: The logic for deleting the concurrency limiter counter from the metric vector and logging a warning if the deletion fails is correctly implemented.

pkg/policies/flowcontrol/actuators/rate-limiter/rate-limiter.go (2)

• 290-295: The UnregisterRateLimiter method is called within the OnStop function to unregister the rate limiter from the data engine. Ensure that the UnregisterRateLimiter method handles potential errors gracefully and that it is safe to call multiple times in case of retries.

• 306-309: The DeletePartialMatch method is used to delete the rate limiter counter from its metric vector. The warning log suggests that there might be cases where no metrics were generated, hence no deletion. This could be a normal scenario, but it's important to ensure that the absence of metrics is not due to an error in the metric generation or collection process.

pkg/policies/flowcontrol/actuators/concurrency-scheduler/concurrency-scheduler.go (1)

• 289-289: The method call UnregisterConcurrencyScheduler is used to unregister the concurrency scheduler from the data engine. It's important to ensure that this method is implemented correctly and handles errors as expected, especially since it's a renaming from a previous method and could have implications on the system's behavior.

pkg/otelcollector/metricsprocessor/processor.go (1)

• 248-248: The log message has been changed from "LoadScheduler not found" to "Scheduler not found". Ensure that this new terminology is consistent throughout the entire codebase to avoid confusion.

Verification successful

---

The search results indicate that the old log message "LoadScheduler not found" is no longer present in the codebase, and the new log message "Scheduler not found" is used in processor.go. This suggests that the terminology change has been consistently applied.

Scripts Executed

The following scripts were executed for the analysis:

---

Script:

#!/bin/bash
# Search for the old log message to ensure it has been updated everywhere.
rg --type go '"LoadScheduler not found"'
# Search for the new log message to confirm its usage.
rg --type go '"Scheduler not found"'

Length of output: 159

[coderabbitai]

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 7a838ad and 5f1135e.

Files selected for processing (1)

• pkg/policies/flowcontrol/actuators/sampler/sampler.go (2 hunks)

Additional comments: 2

pkg/policies/flowcontrol/actuators/sampler/sampler.go (2)

• 259-259: The error message has been corrected to refer to the sampler instead of the rate limiter, aligning with the previous comment and resolution.

• 268-271: The logic for deleting the sampler counter from the metric vector has been added to the OnStop function, with a warning message if no counters are deleted. This change improves the cleanup process by ensuring that metrics are cleaned up when the sampler is stopped.