feat: allow to specify a service.instance.id; fix service.name usage#710
feat: allow to specify a service.instance.id; fix service.name usage#710
Conversation
WalkthroughThis PR threads observability hints through the codebase by introducing an Changes
Sequence Diagram(s)sequenceDiagram
participant App as CLI App
participant Cmd as Command
participant SetupObs as setup_observability()
participant SetupOTel as setup_open_telemetry()
participant Resource as Resource Builder
App->>Cmd: Parse command (e.g., Run)
Cmd->>Cmd: Implement ObservabilityHints
App->>SetupObs: setup_observability(..., &cmd)
SetupObs->>Cmd: hints.listen_address()
Cmd-->>SetupObs: Some("127.0.0.1:3000") or None
SetupObs->>SetupOTel: setup_open_telemetry(..., hints)
SetupOTel->>Resource: Build resource with env vars
Resource->>Resource: Check OTEL_SERVICE_INSTANCE_ID
Resource->>Cmd: If not set, derive from listen_address
Cmd-->>Resource: Address for instance ID fallback
Resource-->>SetupOTel: Complete resource
SetupOTel-->>SetupObs: Tracer configured
SetupObs-->>App: Observability ready
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR makes two changes to the OpenTelemetry observability setup: (1) it fixes the service.name usage so that the tracer respects the OTEL_SERVICE_NAME environment variable instead of always using the hardcoded default, and (2) it adds support for specifying a service.instance.id resource attribute via a new environment variable. It also reduces the visibility of two internal constants from pub to private.
Changes:
- Fix the tracer to use the resolved
service_name(fromOTEL_SERVICE_NAMEenv var or default) instead of the hardcodedDEFAULT_OTLP_SERVICE_NAMEconstant. - Add support for setting
service.instance.idvia aSERVICE_INSTANCE_IDenvironment variable, requiring thesemconv_experimentalfeature flag on theopentelemetry-semantic-conventionscrate. - Make
DEFAULT_OTLP_SERVICE_NAMEandDEFAULT_OTLP_METRIC_URLconstants private (they are only used within the file).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
crates/amaru/src/observability.rs |
Adds SERVICE_INSTANCE_ID env var support, fixes tracer to use resolved service name, makes constants private, imports var and SERVICE_INSTANCE_ID. |
Cargo.toml |
Adds semconv_experimental feature flag to opentelemetry-semantic-conventions dependency to enable SERVICE_INSTANCE_ID. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/amaru/src/observability.rs (1)
320-320: Use a stable instrumentation scope name instead of runtime service config.Right now line 320 is passing the deployment-specific service name to the tracer, but in OpenTelemetry terms, that name should identify the instrumentation scope—basically which code module/library is emitting the telemetry. Using a runtime-dependent value like
OTEL_SERVICE_NAMEcreates scope cardinality drift (think of it like different versions of the same actor playing the same role across different film sets—the character's still the same, but the "scope" keeps fragmenting). Stick with a stable constant like "amaru" so that all deployments report spans under the same instrumentation scope, no matter what they call themselves at runtime.🎯 Suggested patch
+const INSTRUMENTATION_SCOPE_NAME: &str = "amaru"; @@ - let opentelemetry_tracer = opentelemetry_provider.tracer(service_name); + let opentelemetry_tracer = opentelemetry_provider.tracer(INSTRUMENTATION_SCOPE_NAME);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/amaru/src/observability.rs` at line 320, The tracer is being created with a runtime service name (service_name) which causes instrumentation scope cardinality; change the call to use a stable instrumentation scope string (e.g., "amaru") instead of service_name: locate the opentelemetry_tracer instantiation (opentelemetry_provider.tracer(service_name)) and replace the argument with a constant/stable scope name (or a new const INSTRUMENTATION_SCOPE = "amaru") so all spans use a consistent instrumentation scope across deployments.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/amaru/src/observability.rs`:
- Around line 278-283: The service name and instance ID can be empty strings
from env vars and should be treated as unset; update the logic that builds
attributes (the service_name/service_instance_id handling used to populate
attributes Vec and constants SERVICE_NAME and SERVICE_INSTANCE_ID) to ignore
empty or whitespace-only values — e.g., when calling var(...) for
OTEL_SERVICE_NAME and SERVICE_INSTANCE_ID, filter out values where
value.trim().is_empty() and only push KeyValue::new(...) into attributes for
non-empty strings (leave DEFAULT_OTLP_SERVICE_NAME behavior intact by applying
the same emptiness check before adding SERVICE_NAME).
---
Nitpick comments:
In `@crates/amaru/src/observability.rs`:
- Line 320: The tracer is being created with a runtime service name
(service_name) which causes instrumentation scope cardinality; change the call
to use a stable instrumentation scope string (e.g., "amaru") instead of
service_name: locate the opentelemetry_tracer instantiation
(opentelemetry_provider.tracer(service_name)) and replace the argument with a
constant/stable scope name (or a new const INSTRUMENTATION_SCOPE = "amaru") so
all spans use a consistent instrumentation scope across deployments.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 309f741b-1329-4679-85f9-eadc8ea310b6
📒 Files selected for processing (2)
Cargo.tomlcrates/amaru/src/observability.rs
Codecov Report❌ Patch coverage is
... and 9 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Signed-off-by: jeluard <jeluard@users.noreply.github.com>
jeluard
force-pushed
the
jeluard/more-otel-conventions
branch
from
853ff25 to
fee82b1
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
crates/amaru/src/observability.rs (1)
278-283:⚠️ Potential issue | 🟡 MinorTreat empty OTEL env values as unset before building resource attrs.
Line 278 and Line 279 currently accept empty/whitespace values, which can emit blank
service.name/service.instance.idand make telemetry metadata a bit Mad Max in production dashboards.💡 Proposed patch
- let service_name = var("OTEL_SERVICE_NAME").unwrap_or_else(|_| DEFAULT_OTLP_SERVICE_NAME.to_string()); - let service_instance_id = var("OTEL_SERVICE_INSTANCE_ID").ok(); + let service_name = var("OTEL_SERVICE_NAME") + .ok() + .filter(|value| !value.trim().is_empty()) + .unwrap_or_else(|| DEFAULT_OTLP_SERVICE_NAME.to_string()); + let service_instance_id = var("OTEL_SERVICE_INSTANCE_ID") + .ok() + .filter(|value| !value.trim().is_empty());OpenTelemetry SDK environment variable specification: should empty OTEL_SERVICE_NAME values be treated as unset?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/amaru/src/observability.rs` around lines 278 - 283, The code currently accepts empty/whitespace OTEL env values and can emit blank service attributes; update the handling so env values are treated as unset when empty or only whitespace: call var("OTEL_SERVICE_NAME").ok().filter(|s| !s.trim().is_empty()) and then fall back to DEFAULT_OTLP_SERVICE_NAME if absent, and for service_instance_id use var("OTEL_SERVICE_INSTANCE_ID").ok().filter(|s| !s.trim().is_empty()) and only push KeyValue::new(SERVICE_INSTANCE_ID, id) when the filtered Option is Some; keep SERVICE_NAME, SERVICE_INSTANCE_ID and the attributes vector logic otherwise.
🧹 Nitpick comments (1)
crates/amaru/src/observability.rs (1)
320-320: Use stable instrumentation scope name intracer(...), not runtimeservice_name.Right now, line 320 passes
service_name(fromOTEL_SERVICE_NAME) as the tracer scope name. In OTel semantics, that's your instrumentation scope—basically the "who made this telemetry" bit—not the service identity. By tying it to env config, you're creating a new scope for every service instance, which balloons cardinality like a bad video game graphics setting and fragments your observability.The
service_nameis already being set correctly in your Resource attributes (where it belongs), so just swap it out with a stable crate identifier:🎯 Suggested refactor
+const INSTRUMENTATION_SCOPE_NAME: &str = env!("CARGO_PKG_NAME"); @@ - let opentelemetry_tracer = opentelemetry_provider.tracer(service_name); + let opentelemetry_tracer = opentelemetry_provider.tracer(INSTRUMENTATION_SCOPE_NAME);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/amaru/src/observability.rs` at line 320, The tracer scope is being created with the runtime service_name (OTEL_SERVICE_NAME) which raises cardinality; change the opentelemetry_provider.tracer(...) call that assigns opentelemetry_tracer to use a stable instrumentation scope identifier (e.g., the crate name "amaru" or a constant like CRATE_NAME) instead of the dynamic service_name variable; leave the Resource attributes as-is (service_name stays in Resource) and update the tracer(...) invocation in observability.rs accordingly so the instrumentation scope is stable across instances.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@crates/amaru/src/observability.rs`:
- Around line 278-283: The code currently accepts empty/whitespace OTEL env
values and can emit blank service attributes; update the handling so env values
are treated as unset when empty or only whitespace: call
var("OTEL_SERVICE_NAME").ok().filter(|s| !s.trim().is_empty()) and then fall
back to DEFAULT_OTLP_SERVICE_NAME if absent, and for service_instance_id use
var("OTEL_SERVICE_INSTANCE_ID").ok().filter(|s| !s.trim().is_empty()) and only
push KeyValue::new(SERVICE_INSTANCE_ID, id) when the filtered Option is Some;
keep SERVICE_NAME, SERVICE_INSTANCE_ID and the attributes vector logic
otherwise.
---
Nitpick comments:
In `@crates/amaru/src/observability.rs`:
- Line 320: The tracer scope is being created with the runtime service_name
(OTEL_SERVICE_NAME) which raises cardinality; change the
opentelemetry_provider.tracer(...) call that assigns opentelemetry_tracer to use
a stable instrumentation scope identifier (e.g., the crate name "amaru" or a
constant like CRATE_NAME) instead of the dynamic service_name variable; leave
the Resource attributes as-is (service_name stays in Resource) and update the
tracer(...) invocation in observability.rs accordingly so the instrumentation
scope is stable across instances.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e7715ead-8447-4801-91a1-82605511e592
📒 Files selected for processing (3)
Cargo.tomlcrates/amaru/src/observability.rsmonitoring/README.md
✅ Files skipped from review due to trivial changes (1)
- monitoring/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- Cargo.toml
Signed-off-by: jeluard <jeluard@users.noreply.github.com>
Signed-off-by: jeluard <jeluard@users.noreply.github.com>
jeluard
force-pushed
the
jeluard/more-otel-conventions
branch
from
34cdf9e to
fe325e8
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
crates/amaru/src/observability.rs (1)
289-290:⚠️ Potential issue | 🟡 MinorBlank
OTEL_SERVICE_NAMEshould be treated as unset.Line 289 trims the env var but still accepts
OTEL_SERVICE_NAME=as"". The OpenTelemetry env-var spec says empty values must be handled the same as unset, so this should fall back instead of emitting a blankservice.name. Same goblin as before, just still hiding in the cave. (opentelemetry.io)OpenTelemetry SDK environment variable specification empty OTEL_SERVICE_NAME treated as unset🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/amaru/src/observability.rs` around lines 289 - 290, The current assignment to service_name uses var("OTEL_SERVICE_NAME").unwrap_or_else(...).trim().to_string() which accepts an empty trimmed string as a valid value; change it to treat empty/whitespace-only values as unset by checking the env var after trimming and falling back to DEFAULT_OTLP_SERVICE_NAME when the trimmed value is empty (e.g., get the env var string, trim it, and if result.is_empty() use DEFAULT_OTLP_SERVICE_NAME.to_string() else use the trimmed value) so service.name is never set to a blank string.
🧹 Nitpick comments (1)
crates/amaru/src/observability.rs (1)
341-341: Sync instrumentation scope with crate identity, not runtime service config.The
TracerProvider::tracer(name)call sets the instrumentation scope—which should stay locked to your library or crate name, not drift with deployment config. Think of it like the credits rolling (your crate name stays the same), whileservice.nameis the character you're playing (changes per deployment). According to OpenTelemetry spec, instrumentation scope identifies which library produced the telemetry, whileservice.namegoes in the Resource for identifying which service is running. Right now you're mixing those two—like keeping your player character name in the credits and your actor name in the character bio.Swap
service_namehere for something stable likeenv!("CARGO_PKG_NAME")to keep the concepts clean.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/amaru/src/observability.rs` at line 341, The instrumentation scope is being set with the runtime service_name via opentelemetry_provider.tracer(service_name), but per OpenTelemetry the tracer (TracerProvider::tracer) should use a stable crate/library identity; change the tracer call to use the crate name (e.g. env!("CARGO_PKG_NAME") or a const CRATE_NAME) instead of service_name so the instrumentation scope stays constant, while keeping service_name in the Resource configuration for service.name; update the call site (opentelemetry_provider.tracer(...)) and any related variable names to reflect the crate identity.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/amaru/src/observability.rs`:
- Around line 289-305: Detected resource attributes from EnvResourceDetector are
being overwritten by your unconditional with_attributes() call; instead call the
default detector first (use Resource::detect() / default resource detection so
EnvResourceDetector runs) to get detected_resource, then build the final
resource by starting from detected_resource and only add SERVICE_NAME (from
service_name) if detected_resource does not already contain SERVICE_NAME, and
only add SERVICE_INSTANCE_ID if neither detected_resource nor the explicit
service_instance_id override exists; use Resource::builder()/with_attributes()
on the detected resource or merge helpers to add missing KeyValue entries and
assign that to resource (refer to Resource::detect/detected_resource,
SERVICE_NAME, SERVICE_INSTANCE_ID, service_name, service_instance_id, and
resource variables).
---
Duplicate comments:
In `@crates/amaru/src/observability.rs`:
- Around line 289-290: The current assignment to service_name uses
var("OTEL_SERVICE_NAME").unwrap_or_else(...).trim().to_string() which accepts an
empty trimmed string as a valid value; change it to treat empty/whitespace-only
values as unset by checking the env var after trimming and falling back to
DEFAULT_OTLP_SERVICE_NAME when the trimmed value is empty (e.g., get the env var
string, trim it, and if result.is_empty() use
DEFAULT_OTLP_SERVICE_NAME.to_string() else use the trimmed value) so
service.name is never set to a blank string.
---
Nitpick comments:
In `@crates/amaru/src/observability.rs`:
- Line 341: The instrumentation scope is being set with the runtime service_name
via opentelemetry_provider.tracer(service_name), but per OpenTelemetry the
tracer (TracerProvider::tracer) should use a stable crate/library identity;
change the tracer call to use the crate name (e.g. env!("CARGO_PKG_NAME") or a
const CRATE_NAME) instead of service_name so the instrumentation scope stays
constant, while keeping service_name in the Resource configuration for
service.name; update the call site (opentelemetry_provider.tracer(...)) and any
related variable names to reflect the crate identity.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 225bc29f-6899-437a-8293-a6a812e7bd1f
📒 Files selected for processing (4)
crates/amaru/src/bin/amaru/cmd/run.rscrates/amaru/src/bin/amaru/main.rscrates/amaru/src/bin/ledger/main.rscrates/amaru/src/observability.rs
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/amaru/src/observability.rs`:
- Around line 290-296: The port extraction from hints.listen_address() in the
service_instance_id computation can misinterpret bare IPv6 like "::1"
(rsplit(':').next() yields "1"); update the logic used in the closure for
service_instance_id to robustly parse the listen address: attempt to parse
listen_addr as a std::net::SocketAddr (or detect a ']' indicating bracketed IPv6
and split on the final ':' only when a numeric port is present) and extract the
port from that parsed SocketAddr, otherwise return None; adjust the closure that
currently calls hints.listen_address() and rsplit to use this safer parsing
strategy so service_instance_id is not built from an incorrect "port".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 939bb23e-3f7b-4ec3-addf-8c9f3c048607
📒 Files selected for processing (4)
crates/amaru/src/bin/amaru/cmd/run.rscrates/amaru/src/bin/amaru/main.rscrates/amaru/src/bin/ledger/main.rscrates/amaru/src/observability.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- crates/amaru/src/bin/amaru/main.rs
- crates/amaru/src/bin/amaru/cmd/run.rs
Signed-off-by: jeluard <jeluard@users.noreply.github.com>
jeluard
force-pushed
the
jeluard/more-otel-conventions
branch
from
7fcefa1 to
14cf623
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
crates/amaru/src/observability.rs (2)
294-300:⚠️ Potential issue | 🟡 MinorIPv6 edge case with bare addresses — the port extraction could misfire.
The
rsplit(':').next()approach works grand for typical formats like192.168.1.1:8080or bracketed IPv6[::1]:8080, but if someone passes a bare IPv6 like::1without a port, you'd extract"1"as the "port" — bit of a plot twist there, like a hidden boss appearing where you least expect it.Since this is a fallback mechanism and typical listen addresses should be well-formed
host:portstrings, it's not a showstopper. But worth considering a more robust parse, or at minimum documenting the expected format.🔧 One approach using SocketAddr parsing
- let service_instance_id: Option<String> = - var("OTEL_SERVICE_INSTANCE_ID").ok().map(|v| v.trim().to_string()).filter(|v| !v.is_empty()).or_else(|| { - let listen_addr = hints.listen_address()?; - let hostname = sysinfo::System::host_name().unwrap_or_else(|| "localhost".to_string()); - let port = listen_addr.trim().rsplit(':').next()?; - Some(format!("{hostname}:{port}")) - }); + let service_instance_id: Option<String> = + var("OTEL_SERVICE_INSTANCE_ID").ok().map(|v| v.trim().to_string()).filter(|v| !v.is_empty()).or_else(|| { + let listen_addr = hints.listen_address()?; + let hostname = sysinfo::System::host_name().unwrap_or_else(|| "localhost".to_string()); + // Parse as SocketAddr to handle IPv6 correctly + let port = listen_addr + .trim() + .parse::<std::net::SocketAddr>() + .ok() + .map(|addr| addr.port().to_string())?; + Some(format!("{hostname}:{port}")) + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/amaru/src/observability.rs` around lines 294 - 300, The fallback port extraction from listen_addr using rsplit(':').next() can misinterpret bare IPv6 addresses (e.g., "::1") as a port; update the logic in the service_instance_id construction (the block using hints.listen_address(), listen_addr, and sysinfo::System::host_name()) to robustly parse the address: attempt to parse listen_addr as a std::net::SocketAddr (or handle bracketed IPv6) and extract the port from that parsed SocketAddr, falling back gracefully if parsing fails, then format the instance id as "{hostname}:{port}". Ensure you reference and update the existing service_instance_id/or_else closure that currently uses rsplit(':').next().
292-293:⚠️ Potential issue | 🟡 MinorEmpty
OTEL_SERVICE_NAMEsneaks through — spec says treat it as unset.G'day mate, the OTel SDK spec is clear: empty environment variables should be handled the same as unset. Right now, if someone sets
OTEL_SERVICE_NAME="", you'll end up with an empty string as your service name — which is like naming your character in a game with just spaces and watching chaos unfold in the leaderboards.You've got the right pattern for
service_instance_idon lines 294-295 with the.filter(|v| !v.is_empty())check, butservice_namehere takes a different path and misses that filter.🛡️ Suggested fix to filter empty strings
- let service_name = - var("OTEL_SERVICE_NAME").unwrap_or_else(|_| DEFAULT_OTLP_SERVICE_NAME.to_string()).trim().to_string(); + let service_name = var("OTEL_SERVICE_NAME") + .ok() + .map(|v| v.trim().to_string()) + .filter(|v| !v.is_empty()) + .unwrap_or_else(|| DEFAULT_OTLP_SERVICE_NAME.to_string());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/amaru/src/observability.rs` around lines 292 - 293, The OTEL service_name currently accepts an empty string when OTEL_SERVICE_NAME is set to "", so update the retrieval logic for service_name (the let service_name = var("OTEL_SERVICE_NAME")... line that uses DEFAULT_OTLP_SERVICE_NAME) to mirror the service_instance_id pattern: after calling var("OTEL_SERVICE_NAME"), add a .filter(|v| !v.is_empty()) (or equivalent check) before unwrap_or_else so that empty values are treated as unset and DEFAULT_OTLP_SERVICE_NAME is used instead; reference the existing service_instance_id handling as the model for the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/amaru/src/observability.rs`:
- Around line 302-309: The current logic (using default_resource from
Resource::builder().build()) incorrectly preserves OTEL_RESOURCE_ATTRIBUTES over
OTEL_SERVICE_NAME; change the behavior so ENV-derived OTEL_SERVICE_NAME
(service_name) overrides any existing service.name in default_resource: when
service_name is present, remove or replace any existing SERVICE_NAME key in
default_resource before pushing KeyValue::new(SERVICE_NAME,
service_name.clone()) (i.e., do not skip adding just because default_resource
has the key). Do the same for service_instance_id/SERVICE_INSTANCE_ID: if
service_instance_id is Some, replace any existing key from default_resource
rather than skipping. Update the code around default_resource, attributes,
SERVICE_NAME, service_name, SERVICE_INSTANCE_ID, and service_instance_id to
perform replace/overwrite semantics instead of preserving existing values.
---
Duplicate comments:
In `@crates/amaru/src/observability.rs`:
- Around line 294-300: The fallback port extraction from listen_addr using
rsplit(':').next() can misinterpret bare IPv6 addresses (e.g., "::1") as a port;
update the logic in the service_instance_id construction (the block using
hints.listen_address(), listen_addr, and sysinfo::System::host_name()) to
robustly parse the address: attempt to parse listen_addr as a
std::net::SocketAddr (or handle bracketed IPv6) and extract the port from that
parsed SocketAddr, falling back gracefully if parsing fails, then format the
instance id as "{hostname}:{port}". Ensure you reference and update the existing
service_instance_id/or_else closure that currently uses rsplit(':').next().
- Around line 292-293: The OTEL service_name currently accepts an empty string
when OTEL_SERVICE_NAME is set to "", so update the retrieval logic for
service_name (the let service_name = var("OTEL_SERVICE_NAME")... line that uses
DEFAULT_OTLP_SERVICE_NAME) to mirror the service_instance_id pattern: after
calling var("OTEL_SERVICE_NAME"), add a .filter(|v| !v.is_empty()) (or
equivalent check) before unwrap_or_else so that empty values are treated as
unset and DEFAULT_OTLP_SERVICE_NAME is used instead; reference the existing
service_instance_id handling as the model for the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9be47d56-8978-43f9-a66a-32cb0eac9f61
📒 Files selected for processing (6)
Cargo.tomlcrates/amaru/src/bin/amaru/cmd/run.rscrates/amaru/src/bin/amaru/main.rscrates/amaru/src/bin/ledger/main.rscrates/amaru/src/observability.rsmonitoring/README.md
Signed-off-by: jeluard <jeluard@users.noreply.github.com>
Summary by CodeRabbit
Documentation
Refactor