Skip to content

Refactor report() to use keyword arguments with predefined units#3

Merged
nateberkopec merged 3 commits intomainfrom
hash-based-report-signature
Nov 1, 2025
Merged

Refactor report() to use keyword arguments with predefined units#3
nateberkopec merged 3 commits intomainfrom
hash-based-report-signature

Conversation

@nateberkopec
Copy link
Copy Markdown
Contributor

@nateberkopec nateberkopec commented Nov 1, 2025

Summary

Refactors the report() method to use proper keyword arguments with a cleaner signature:

Before:

reporter.report("QueueLatency", 42, integration: :sidekiq, unit: "Seconds", dimensions: [{name: "QueueName", value: "default"}])

After:

reporter.report(metric: :QueueLatency, value: 42, dimensions: {QueueName: "default"})

Changes

New Signature

  • metric: - Required symbol for the metric name
  • value: - Required value for the metric
  • dimensions: - Optional hash of dimensions (no more array of hashes!)
  • namespace: - Optional string to override the default namespace

Predefined Units

Units are now predefined in the configuration and cannot be overridden. Each metric has a fixed unit that is automatically applied:

  • Count metrics: Workers, EnqueuedJobs, QueueSize, etc.
  • Time metrics: QueueLatency (Seconds), RequestQueueTime (Milliseconds)
  • Percentage metrics: Utilization

This ensures consistency and prevents accidental misuse of units.

Implementation Details

  • Added units configuration hash to Configuration class mapping each metric to its unit
  • Updated MetricReporter#report to use proper kwargs signature
  • Integration is automatically determined from the metric name via config
  • Updated all adapters (Sidekiq, Puma, ActiveJob, Rack) to use new signature
  • Updated all test files and test doubles to match

Benefits

  • Clearer API: Proper keyword arguments instead of positional or hash-key based
  • Type safety: Metric name is explicitly a symbol
  • Hash dimensions: Much cleaner than array of hashes
  • Unit consistency: Units are predefined and cannot be accidentally changed
  • Less repetition: No need to specify units on every call

Test Plan

  • All 68 tests pass
  • Lint checks pass (flog, flay, standard)
  • Updated all adapters and test files
  • Added unit mappings to configuration

🤖 Generated with Claude Code

nateberkopec and others added 2 commits November 1, 2025 12:47
Changes the report() method signature from positional arguments to a hash-based approach where the metric name is the key.

The method now automatically looks up the integration based on the metric name from the config, eliminating the need to pass it explicitly. Dimensions can be provided as a hash for cleaner syntax.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Changes the report() method to use proper keyword arguments:
- metric: (required symbol)
- value: (required)
- dimensions: (optional hash)
- namespace: (optional string)

Units are now predefined in configuration and cannot be overridden. Each metric has a fixed unit that is automatically applied based on the metric name.

Added unit mappings to Configuration for all metrics (Count, Seconds, Milliseconds, Percent).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@nateberkopec nateberkopec changed the title Refactor report() to use hash-based signature Refactor report() to use keyword arguments with predefined units Nov 1, 2025
Simplifies dimension handling by requiring dimensions to always be a hash. Removes the convert_dimensions method and the unnecessary conditional logic for handling arrays or other types.

Dimensions are now always expected as a hash and converted inline.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@nateberkopec nateberkopec merged commit 1fddf03 into main Nov 1, 2025
3 of 7 checks passed
@nateberkopec nateberkopec deleted the hash-based-report-signature branch November 4, 2025 03:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant