Skip to content

refactor: rename count to metrics and move location#21

Merged
nnshah1 merged 10 commits into
mainfrom
nnshah1-refactor-count-location
Mar 7, 2025
Merged

refactor: rename count to metrics and move location#21
nnshah1 merged 10 commits into
mainfrom
nnshah1-refactor-count-location

Conversation

@nnshah1

@nnshah1 nnshah1 commented Mar 5, 2025

Copy link
Copy Markdown
Contributor

Overview:

Refactor location of metrics to components folder.

Details:

  • change name of application from count to metrics
  • move metrics to components folder
  • add metrics services to common deploy/docker-compose.yml

Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)

  • closes GitHub issue: #xxx

@github-actions

github-actions Bot commented Mar 5, 2025

Copy link
Copy Markdown
Contributor

Test Results

  2 files    2 suites   54s ⏱️
 80 tests  80 ✅ 0 💤 0 ❌
102 runs  101 ✅ 1 💤 0 ❌

Results for commit b5126d2.

♻️ This comment has been updated with latest results.

@nnshah1 nnshah1 requested a review from a team March 5, 2025 07:34
@biswapanda

Copy link
Copy Markdown
Contributor

alternative names:

  • metrics_collector (not too bad- google search shows similar uses)
  • metrics_aggregator (too long)
  • aggregator (too generic)
  • metrics (overloaded with well known concepts of metrics)

Comment thread deploy/metrics/README.md Outdated

@rmccorm4 rmccorm4 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Rename LGTM - needs merge conflicts resolved, and see comment on dashboard/ directory

@nnshah1

nnshah1 commented Mar 6, 2025

Copy link
Copy Markdown
Contributor Author

alternative names:

  • metrics_collector (not too bad- google search shows similar uses)
  • metrics_aggregator (too long)
  • aggregator (too generic)
  • metrics (overloaded with well known concepts of metrics)

will go with metrics for now and disambiguate in documentation

@nnshah1 nnshah1 enabled auto-merge (squash) March 6, 2025 07:24
@nnshah1 nnshah1 merged commit 2585750 into main Mar 7, 2025
@nnshah1 nnshah1 deleted the nnshah1-refactor-count-location branch March 7, 2025 01:28
kylehh pushed a commit to kylehh/dynamo that referenced this pull request Apr 11, 2025
kangclzjc added a commit to kangclzjc/dynamo that referenced this pull request Jun 4, 2026
…oder reuse, doc fixes

Low-risk cleanup batch from the independent review (no decision-path change):

- ai-dynamo#4 chain_augment: add ``predicted_kv_hit_rate`` to ``_PREDICTION_FIELDS``
  so it participates in first-writer-wins partial merge like the other three
  predicted_* fields (was silently dropped in any 2+ plugin PREDICT chain,
  contradicting the proto/Pydantic contract). +2 chain_augment tests.
- #10 engine_adapter: add ``scale_down_capped_by_throughput`` to
  ``_aggregate_disagg_load_reason`` priority (PSM disagg emits it; placed
  between scale_up and scale_down to mirror PSM's _PRIORITY).
- ai-dynamo#11 dead code: drop ``contributing_plugin_ids`` (built, never read) in
  pipeline._run_fanout_stage; drop ``_set_enabled`` + ``_plugin_ids``
  (no caller in PR #1; would KeyError if reached).
- ai-dynamo#18 _encode_fpm: use the canonical
  ``dynamo.common.forward_pass_metrics.encode`` (shared module-level encoder)
  instead of allocating a fresh ``msgspec.msgpack.Encoder`` per tick and
  re-implementing the encoding. Byte-identical wire format; keeps FPM
  serialization in lock-step with the rest of dynamo.
- ai-dynamo#17 transport ABC docstring: timeout is enforced by the transport
  (``call()`` wraps ``asyncio.wait_for``), not the orchestrator — the
  pipeline uses a bare gather to avoid double-counting the deadline.
- ai-dynamo#20 scheduler docstring: note the heartbeat-eviction monitor is not wired
  in this PR (last_heartbeat_at is recorded but unread; monitor is follow-up).
- ai-dynamo#21 transport contract test: 7 inputs (not 8) → 14 cases (multi_pool fixture
  was removed with component_name; comments were stale).
- ai-dynamo#22 metrics test: remove the dead no-op ``pass`` loop in _sample_value.

828 planner tests pass (was 825; +3 chain-augment / merge tests).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.

5 participants