Use app as the default service name - #270
Conversation
|
Rationale for this (and for consistency, its twin Ruby PR at appsignal/appsignal-ruby#1581) is to have the same name for customers who upgrade from agent mode to collector mode. @matsimitsu will be changing the processor's translation layer to use "app" as well, so we have a shared, reasonable enough default. Docs will still advise setting a service name as part of collector mode setup, so the fallback may not be very relevant, but customers who upgrade somewhat manually may skip the docs. |
2b7e126 to
cf40f4b
Compare
appapp as the default service name
There was a problem hiding this comment.
Pull request overview
Updates the OpenTelemetry default service name from unknown to app.
Changes:
- Changes the service-name fallback to
app. - Updates the corresponding test.
- Adds a patch changeset.
A moderate issue remains in src/appsignal/opentelemetry.py: Options(service_name=None) does not emit app because the present None value bypasses the dictionary fallback.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Summary |
|---|---|
tests/test_config.py |
Updates the expected default service name. |
src/appsignal/opentelemetry.py |
Changes the service-name fallback; requires a null-aware fallback for explicit None. |
.changesets/default-service-name-to-app.md |
Documents the patch release. |
Suppressed comments (2)
src/appsignal/opentelemetry.py:293
_resourceis also used for the embedded-agent path:Client.start()callsstart_opentelemetryafter starting either the agent or the external collector, whileservice_nameis explicitly collector-only inConfig._warn_collector_exclusive_options(config.py:429-440). This unconditional fallback therefore changes the agent-mode resource as well, despite the changeset and description scoping the behavior to collector mode. Either gate the new fallback onconfig.should_use_collector()and preserve the existing agent-mode default, or broaden the documented scope and tests.
"service.name": config.options.get("service_name", "app"),
tests/test_config.py:344
- This test builds
Config(Options()), which has no collector endpoint and therefore exercises the agent path, but the changeset says the new fallback is collector-only. Since_resourceis also used for agent tracer/metric providers, this assertion locks in behavior outside the stated scope and leaves the collector case untested. Either make this a collector config and add/retain an agent-mode assertion for"unknown", or update the changeset to describe a global change.
assert resource.attributes["service.name"] == "app"
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Replies from @unflxw, added with copilot-loop. Copilot's review above is unchanged.
src/appsignal/opentelemetry.py:293 — Dismissed. The service_name option only has an effect in collector mode, which is why Config._warn_collector_exclusive_options warns when it is set without a collector endpoint. The agent ignores the service.name resource attribute, so the value the resource carries in agent mode makes no difference to what a customer sees. The changeset describes where the change is visible, which is collector mode.
tests/test_config.py:344 — Dismissed. _resource builds the same attributes whichever mode is in use, so a test that does not set a collector endpoint still covers the fallback. There is no separate collector-mode behaviour to test here, and the value only reaches a customer in collector mode, which is what the changeset says.
The collector turns the service name into names that customers see. A
trace's namespace becomes `{service name}/{namespace}`, so a web
request from an app that does not set `service_name` was filed under
`unknown/web`. A log line's group falls back to the service name on
its own, so those lines were grouped under `unknown`.
Leaving `service_name` unset is the normal case for an app that is a
single service. Nothing about it is actually unknown, so the fallback
now describes what it is. The `revision` and `hostname` fallbacks
still report `unknown`, because those are facts about the running
process that AppSignal really does not have.
cf40f4b to
fcf33d8
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/appsignal/opentelemetry.py:293
- This fallback is applied in agent mode as well as collector mode:
_resourceis used by_start_tracerand_start_metricsfor both, whileservice_nameis documented as collector-only insrc/appsignal/config.py:430-441. That changes the agent-mode resource fromunknowntoapp, contrary to the changeset's collector-only scope. Gate theappfallback onconfig.should_use_collector()(and retainunknownfor agent mode), or update the documented behavior if the agent change is intentional.
"service.name": config.options.get("service_name") or "app",
Replies from @unflxw, added with copilot-loop. Copilot's review above is unchanged.
src/appsignal/opentelemetry.py:293 — Dismissed, for the same reason as on the previous review. The service_name option only has an effect in collector mode, which is why Config._warn_collector_exclusive_options warns when it is set without a collector endpoint. The agent ignores the service.name resource attribute, so the value the resource carries in agent mode never reaches a customer. Gating the fallback on config.should_use_collector() would add a branch that no customer can observe. The changeset describes where the change is visible, which is collector mode.
The OpenTelemetry resource looks up `revision` and `hostname` with a dictionary default, which only applies when the key is missing. Passing either option as `None` puts the key in the config with a `None` value, so the lookup returns that `None` and the resource builder drops the attribute. The collector supplies its own revision when the attribute is absent, but not its own host name, so an app configured with `hostname=None` reported no host name at all.
d5d44b0 to
3e289ce
Compare
Use
appas the default service nameThe collector turns the service name into names that customers see. A
trace's namespace becomes
{service name}/{namespace}, so a webrequest from an app that does not set
service_namewas filed underunknown/web. A log line's group falls back to the service name onits own, so those lines were grouped under
unknown.Leaving
service_nameunset is the normal case for an app that is asingle service. Nothing about it is actually unknown, so the fallback
now describes what it is. The
revisionandhostnamefallbacksstill report
unknown, because those are facts about the runningprocess that AppSignal really does not have.
Apply revision and host name fallbacks to None
The OpenTelemetry resource looks up
revisionandhostnamewith adictionary default, which only applies when the key is missing. Passing
either option as
Noneputs the key in the config with aNonevalue,so the lookup returns that
Noneand the resource builder drops theattribute. The collector supplies its own revision when the attribute is
absent, but not its own host name, so an app configured with
hostname=Nonereported no host name at all.