Skip to content

Add structured metrics instrumentation #12230

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
Feb 28, 2025
Merged

Conversation

vittoriopolverino
Copy link
Member

@vittoriopolverino vittoriopolverino commented Feb 5, 2025

Motivation

The existing Counter implementation follows a parsing-based approach, relying on metric name patterns like label1:label2:* and inferring implicit relationships through JSON structures, dictionaries, or other nested data formats. Since these structures can vary depending on the feature or how each feature owner decides to implement them, querying, filtering, and aggregating metrics become inconsistent and complex. This lack of explicit structure makes it harder to standardize analysis across different use cases.

This refactor introduces a structured and extensible approach to metric collection, enforcing clear label standards and dimensional analysis while making all relationships and classifications explicit, rather than relying on naming conventions, positional placement, or nested JSON structures. By ensuring consistency, we can simplify querying, filtering, and aggregations, making analytics more scalable and reducing the need for custom metric handling.

Changes

  • Moved metrics to metrics.py, making them a standalone concept, not strictly tied to usage.py.
  • Introduced MetricRegistry as a singleton for managing all registered metrics (Counters, Gauges, etc.).
  • Introduced Metric as the base interface for metrics, enforcing the implementation of the collect() method in all metric types (e.g., Counters, Gauges).
  • Updated published event name from ls:usage to ls_metrics
  • Set a limit of 8 labels per metric to ensure labels are used effectively as dimensions for time-series analysis, filtering, and aggregations, preventing misuse or unnecessary complexity. A dedicated guide will follow to outline best practices.

Usage Examples

Basic Counter
A Counter tracks the occurrences of an event. It can be used without labels for simple counting:

from localstack.utils.analytics.metrics import Counter

# Define a counter
http_responses = Counter(namespace="http", name="requests")

# Increment
http_responses.increment()
http_responses.increment(value=3)

# Reset
http_responses.reset()

Counters with Labels
Counters can include labels to categorize events more effectively:

from localstack.utils.analytics.metrics import Counter

# Define a counter with labels
chaos_invocations = Counter(namespace="chaos", name="invocations", labels=["operation"])

# Increment
chaos_invocations.labels(operation="fault").increment(value=3)
chaos_invocations.labels(operation="network-effect").increment(value=4)

# Reset
chaos_invocations.labels(operation="fault").reset()
chaos_invocations.labels(operation="network-effect").reset()

Testing

Unit Tests (No External Services Required)

  • The test suite includes unit tests that validate metric collection and registry behavior without requiring event publishing to the analytics backend.

End-to-End Testing with the Analytics Backend

For a more in-depth test, follow these steps:

  1. Run the Analytics Backend Locally
  • Start the analytics backend to capture and process metric events.
  • Ensure that environment variables are correctly set to send events to the data platform.
  1. Start LocalStack Core with Analytics Enabled
  • Start LocalStack Core with the appropriate environment variables to enable analytics and send events to the local backend:
ANALYTICS_API="http://localhost:8000/v1" \
DEBUG=1 \
DEBUG_ANALYTICS=1 \
python -m localstack.cli.main start --host

TODO

What's left to do:

  • Instrument Chaos API - PR.
  • Complete internal documentation on best practices for metrics and label naming

@vittoriopolverino vittoriopolverino added this to the 4.2 milestone Feb 5, 2025
Copy link
Collaborator

@localstack-bot localstack-bot left a comment

Choose a reason for hiding this comment

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

Welcome to LocalStack! Thanks for raising your first Pull Request and landing in your contributions. Our team will reach out with any reviews or feedbacks that we have shortly. We recommend joining our Slack Community and share your PR on the #community channel to share your contributions with us. Please make sure you are following our contributing guidelines and our Code of Conduct.

@localstack-bot
Copy link
Collaborator

localstack-bot commented Feb 5, 2025

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

Copy link

github-actions bot commented Feb 5, 2025

LocalStack Community integration with Pro

    2 files  ±0      2 suites  ±0   1h 54m 20s ⏱️ + 4m 21s
4 104 tests +1  3 772 ✅ +2  332 💤  - 1  0 ❌ ±0 
4 106 runs  +1  3 772 ✅ +2  334 💤  - 1  0 ❌ ±0 

Results for commit 0691017. ± Comparison against base commit f5b247b.

♻️ This comment has been updated with latest results.

@vittoriopolverino vittoriopolverino changed the title Add structured Metrics Instrumentation Add structured metrics instrumentation Feb 5, 2025
@vittoriopolverino vittoriopolverino added the semver: patch Non-breaking changes which can be included in patch releases label Feb 7, 2025
@vittoriopolverino
Copy link
Member Author

I have read the CLA Document and I hereby sign the CLA

@vittoriopolverino vittoriopolverino marked this pull request as ready for review February 7, 2025 11:37
@vittoriopolverino vittoriopolverino marked this pull request as draft February 7, 2025 11:39
@vittoriopolverino vittoriopolverino marked this pull request as ready for review February 7, 2025 12:06
Copy link
Member

@alexrashed alexrashed left a comment

Choose a reason for hiding this comment

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

Awesome! This is a really great push for the analytics clients such that they provide a clear interface for common use cases, which will make it easier for us to use while making sure that the structure of the events being sent is nicely digestible 😄
I had a few questions and suggestions to follow up, happy to discuss them directly if you want! 👥 🚀

@vittoriopolverino
Copy link
Member Author

vittoriopolverino commented Feb 18, 2025

question: I'm considering whether to replace exceptions with logs entirely or use them only in specific cases (such as during instantiation or when debug mode is enabled) to reduce as much as possible forcibly interrupting the emulator due to metrics instrumentation. @alexrashed, what’s your take on this?

Copy link
Member

@alexrashed alexrashed left a comment

Choose a reason for hiding this comment

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

Thanks a lot for acting on the comments from the previous reviews! A lot of the comments are resolved. I don't really see the necessity of the complex structure between the different Counter classes, but that shouldn't be a blocker.
I also added some more comments to discuss.

I feel like a lot of the complexity is coming from the need to improve the developer experience when using the new module, but then there are no real-world usages yet.
I think it is crucial to see this in action by migrating existing callers of the usage.py module. This will make it way more clearer how this framework performs in terms of developer experience. 😄

@alexrashed alexrashed force-pushed the add-metrics-counter branch 2 times, most recently from b7f8425 to 36bdb0b Compare February 24, 2025 15:22
@vittoriopolverino vittoriopolverino modified the milestones: 4.2, 4.3 Feb 25, 2025
Copy link
Member

@alexrashed alexrashed left a comment

Choose a reason for hiding this comment

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

I just added a few nitpick comments here, but from my perspective we are ready to roll!
Given that this is targeted for 4.3, and you might want to iterate over it one more time, we should be ready to merge this after the 4.2 release tomorrow! 🚀

…d empty check for name attribute, adjusted data tupe for _label_values and _counters_by_label_values
@vittoriopolverino
Copy link
Member Author

FYI: I'm fixing unit-tests right now!

@vittoriopolverino vittoriopolverino added semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases and removed semver: patch Non-breaking changes which can be included in patch releases labels Feb 27, 2025
@localstack localstack deleted a comment from localstack-bot Feb 28, 2025
@vittoriopolverino vittoriopolverino merged commit 17d852a into master Feb 28, 2025
36 of 38 checks passed
@vittoriopolverino vittoriopolverino deleted the add-metrics-counter branch February 28, 2025 08:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants