Test PR for automation testing - #640
Conversation
Adds the empty crate skeleton (Cargo.toml, lib.rs, README) and a design document (docs/DESIGN.md) describing the rescaling meter-provider wrapper and its open questions, for review before implementation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fold reviewer decisions into the design: round-and-saturate integer rescaling, scale explicit histogram boundaries (no-op for SDK defaults), mandatory sidecar unit, accepted observable double invocation, name-only scope matching with room for future exact matching, multiplication-only transform, build-time validation via panic, and a type-erased by-value inner provider. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Wraps an inner OpenTelemetry meter provider and, for instruments configured per scope at build time, transparently emits rescaled side-by-side sidecar instruments (e.g. a seconds histogram gains a millis sidecar scaled by 1000). Supports all sync and observable instrument kinds, scales histogram bucket boundaries, rounds and saturates integer rescales, and passes unconfigured scopes/instruments straight through. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…mple Move test-only lint allows onto the affected test modules, condense API doc summary lines to a single line, shorten item paths (fmt::Debug, PhantomData), derive Debug struct names from type_name(), and use foldhash-backed maps for the scope/rule lookups. Adds a runnable print_metrics example that shows the rescaled sidecars next to their originals and a metric with no sidecar. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add a placeholder logo.png (kept out of the package include allowlist) so the README image resolves, and correct the README heading and design-doc title to the standard OpenTelemetry capitalization. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…esign doc The public API exposes only MeterProvider and Meter (KeyValue is internal only), matching the allowed_external_types allowlist. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sander Saares (sandersaares)
left a comment
Sander Saares (sandersaares)
left a comment
There was a problem hiding this comment.
[Copilot speaking] probe summary: verifying that the review body carries the summary.
Sander Saares (sandersaares)
left a comment
Sander Saares (sandersaares)
left a comment
There was a problem hiding this comment.
[Copilot speaking] probe summary 2
Sander Saares (sandersaares)
left a comment
Sander Saares (sandersaares)
left a comment
There was a problem hiding this comment.
[Copilot speaking]
Submitted 4 findings. One finding follows up on an existing discussion thread.
See diagnostics
| Diagnostic | Value |
|---|---|
| Cache | Miss |
| Instruction expansion | Degraded |
Sander Saares (sandersaares)
left a comment
Sander Saares (sandersaares)
left a comment
There was a problem hiding this comment.
[Copilot speaking]
Submitted 4 findings. One finding follows up on an existing discussion thread.
See diagnostics
| Diagnostic | Value |
|---|---|
| Cache | Miss |
| Instruction expansion | Degraded |
Sander Saares (sandersaares)
left a comment
Sander Saares (sandersaares)
left a comment
There was a problem hiding this comment.
[Copilot speaking]
Published 3 findings. No finding follows up on an existing discussion thread.
See diagnostics
| Diagnostic | Value |
|---|---|
| Cache | Miss |
| Instruction expansion | Degraded |
| } | ||
| } | ||
|
|
||
| impl Rescale for u64 { | ||
| #[expect( |
There was a problem hiding this comment.
[Copilot speaking]
Validate the rescaling factor before applying it
The rescaling factor is applied to every recorded measurement without being checked against the range the exporter accepts.
Reproducible reasoning: The factor is read from configuration, which is outside the crate, while the accepted range is a property of the exporter, so nothing connects the two.
Consequence: A factor of zero silently reports every measurement as zero, which reads as a healthy service rather than as a misconfiguration.
Recommended action: Reject a factor outside the accepted range when the configuration is read, so the failure names the setting rather than appearing later as implausible telemetry.
References:
Impacted locations:
crates/opentelemetry_rescaled/src/rescale.rs:20-24
There was a problem hiding this comment.
[Copilot speaking]
Follow up on the earlier observation about this code
The earlier discussion on this thread raised a concern that the current revision does not yet address.
Reproducible reasoning: The thread names a specific behaviour, and the reviewed revision still exhibits it.
Consequence: The concern raised in the earlier round remains unresolved while the thread reads as answered.
Recommended action: Address the behaviour the thread names, or state why the current form is preferred.
Impacted locations:
crates/opentelemetry_rescaled/src/config.rs
There was a problem hiding this comment.
[Copilot speaking]
Document the configuration surface this file defines
The file defines the configuration a caller supplies but does not state which settings are required or what happens when one is omitted. Non-ASCII fidelity is exercised here: ✓ é 日本語.
Reproducible reasoning: A caller reading only this file cannot tell a required setting from an optional one, and the defaults live in a different file.
Consequence: Callers discover a missing required setting at run time rather than when they write the configuration.
Recommended action: State, for each setting, whether it is required and what an omitted value falls back to.
Impacted locations:
crates/opentelemetry_rescaled/src/config.rs
There was a problem hiding this comment.
[Copilot speaking]
Name the boundary this file is responsible for
The file changed by this pull request does not state which concern it owns, so a reader cannot tell what belongs in it.
Reproducible reasoning: The anchor names a line far beyond any hunk the pull request touches, which exercises the downgrade to a file-level comment.
Consequence: Without a stated boundary, unrelated concerns accumulate in the file over time.
Recommended action: State the concern the file owns at the top of the file.
Impacted locations:
crates/opentelemetry_rescaled/src/lib.rs
Sander Saares (sandersaares)
left a comment
Sander Saares (sandersaares)
left a comment
There was a problem hiding this comment.
[Copilot speaking]
Published 4 findings. One finding follows up on an existing discussion thread.
See diagnostics
| Diagnostic | Value |
|---|---|
| Cache | Miss |
| Instruction expansion | Degraded |
| } | ||
| } | ||
|
|
||
| impl Rescale for u64 { | ||
| #[expect( |
There was a problem hiding this comment.
[Copilot speaking]
Validate the rescaling factor before applying it
The rescaling factor is applied to every recorded measurement without being checked against the range the exporter accepts.
Reproducible reasoning: The factor is read from configuration, which is outside the crate, while the accepted range is a property of the exporter, so nothing connects the two.
Consequence: A factor of zero silently reports every measurement as zero, which reads as a healthy service rather than as a misconfiguration.
Recommended action: Reject a factor outside the accepted range when the configuration is read, so the failure names the setting rather than appearing later as implausible telemetry.
References:
Impacted locations:
crates/opentelemetry_rescaled/src/rescale.rs:20-24
There was a problem hiding this comment.
[Copilot speaking]
Document the configuration surface this file defines
The file defines the configuration a caller supplies but does not state which settings are required or what happens when one is omitted. Non-ASCII fidelity is exercised here: ✓ é 日本語.
Reproducible reasoning: A caller reading only this file cannot tell a required setting from an optional one, and the defaults live in a different file.
Consequence: Callers discover a missing required setting at run time rather than when they write the configuration.
Recommended action: State, for each setting, whether it is required and what an omitted value falls back to.
Impacted locations:
crates/opentelemetry_rescaled/src/config.rs
There was a problem hiding this comment.
[Copilot speaking]
Name the boundary this file is responsible for
The file changed by this pull request does not state which concern it owns, so a reader cannot tell what belongs in it.
Reproducible reasoning: The anchor names a line far beyond any hunk the pull request touches, which exercises the downgrade to a file-level comment.
Consequence: Without a stated boundary, unrelated concerns accumulate in the file over time.
Recommended action: State the concern the file owns at the top of the file.
Impacted locations:
crates/opentelemetry_rescaled/src/lib.rs
| } | ||
| } | ||
|
|
||
| impl Rescale for u64 { | ||
| #[expect( |
There was a problem hiding this comment.
[Copilot speaking]
Follow up on the earlier observation about this code
The earlier discussion on this thread raised a concern that the current revision does not yet address.
Reproducible reasoning: The thread names a specific behaviour, and the reviewed revision still exhibits it.
Consequence: The concern raised in the earlier round remains unresolved while the thread reads as answered.
Recommended action: Address the behaviour the thread names, or state why the current form is preferred.
Impacted locations:
crates/opentelemetry_rescaled/src/config.rs
1 participant
No description provided.