feat(traces,metrics): honor OTEL_SERVICE_NAME over APP_NAME, guard nil metrics Logger

[akshat-kumar-singhal]

Description:

OTEL_SERVICE_NAME — and a non-empty service.name entry inside OTEL_RESOURCE_ATTRIBUTES — now sets the OTel resource attribute service.name, overriding APP_NAME, in both pkg/gofr/traces/exporters and pkg/gofr/metrics/exporters.

This reverses the precedence decided in #4206, and deliberately so. That PR kept APP_NAME for one reason: metrics/exporters had already shipped resolving service.name from APP_NAME, so flipping traces alone would have reported one name to the trace backend and another to the metric backend, breaking the trace↔metric join. Moving both signals in one PR is what that review left as the way out, and this is that PR. The standard variables now do what the OpenTelemetry specification says they do.

How it works — resolve once, always set, rather than reordering the resource.Options:

• resolveServiceName(appName, logger) reads resource.Environment(), so the SDK's own ranking (OTEL_SERVICE_NAME ahead of the OTEL_RESOURCE_ATTRIBUTES form) is inherited rather than reimplemented. Environment() runs only the fromEnv detector, so it carries no unknown_service:<exe> default to mistake for an operator's value.
• The option order is unchanged (WithFromEnv() then WithAttributes(attrs...)). Simply moving WithFromEnv() last would be a one-line fix, but it would also let any environment key overwrite framework_version, which the current order protects on purpose. Only the value placed in attrs changes.
• An empty environment value is not an override: OTEL_RESOURCE_ATTRIBUTES="service.name=" parses to a valid attribute with an empty value (sdk/resource/env.go), and shipping that would leave the backend with a nameless service. OTEL_SERVICE_NAME="" is already trimmed to unset by the SDK, so only that form is exposed.
• An override is logged at info level from both packages. Nothing is discarded any more, so a warning would fire on a correct configuration — but an operator still needs to explain a backend showing a name that is not APP_NAME. Both packages log because traces' Build returns the NeverSample provider before buildResource when TRACE_EXPORTER is unset: a metrics-only app, which is the default, never reaches the tracing resource code.

The helper is duplicated per package rather than shared. The two buildResources are already deliberate near-duplicates with divergent bodies (traces omits WithHostID, with a comment saying why), the packages do not import each other, and each defines its own Logger interface — sharing would need a third logger type or an any parameter, and would introduce the first pkg/gofr/internal/... in the tree to save fifteen lines. Drift is mitigated by an identical six-row test table in both packages and a cross-reference comment in each helper.

Left on APP_NAME on purpose, each with a comment: mp.Meter(cfg.AppName, ...) is the instrumentation scope name, which surfaces as the otel_scope_name label on every series — following the service name would silently rename a label on all metrics and break dashboards and recording rules for no conformance gain. The health endpoint, MCP and startup telemetry describe the application rather than the OTel resource, and OTEL_SERVICE_NAME is specified as a resource attribute, so extending it there would be GoFr inventing semantics. The configs reference documents that divergence.

Second change, same area: metrics/exporters.Build now substitutes noopLogger for a nil Logger, matching what #4206 did for traces. buildResource logs unguarded, so Build(ctx, cfg, nil) panicked — even though noopLogger has existed in the package all along, documented as the fallback for "callers that do not supply one", and was never applied. The deprecated exported Prometheus helper is one such caller.

Breaking Changes (if applicable):

No API changes — this is a behaviour change for one configuration. OTEL_SERVICE_NAME is not a new variable: the OpenTelemetry SDK already reads it, and in v1.61.0 both signals picked it up and then silently overwrote it with APP_NAME (metrics via the WithAttributes after WithFromEnv; traces because sdktrace.WithResource merges resource.Environment() first and GoFr's APP_NAME resource over it).

Affected: deployments that set OTEL_SERVICE_NAME (or service.name= in OTEL_RESOURCE_ATTRIBUTES) to a value different from APP_NAME. For example, APP_NAME=orders with OTEL_SERVICE_NAME=orders-prod reported orders before and reports orders-prod after, to both the trace and the metric backend. Both signals change together, so the trace↔metric join is preserved. Unsetting the environment variable restores the previous name. Deployments that set only APP_NAME see no change.

Additional Information:

Mutation-checked by hand; each mutation was applied, the suites run, then reverted:

mutation	result
resolveServiceName reverted to cfg.AppName	6 subtests fail (3 per package)
WithFromEnv() moved after WithAttributes(...)	4 subtests fail (framework_version is not overridable and empty service.name falls back to APP_NAME, per package)
empty-value guard dropped	2 subtests fail (1 per package)
metrics nil-Logger guard dropped	Test_Build_substitutesNoopLoggerForANilLogger panics

Coverage: metrics/exporters 90.4% → 91.1%; traces/exporters 97.2% → 97.1%. The traces figure is arithmetic, not a gap: warnIfEnvServiceNameIgnored (fully covered) was replaced by a slightly shorter resolveServiceName (also fully covered), and the only uncovered statements in buildResource are the pre-existing resource.New error branch and the res == nil fallback, neither of which this PR touches. All new code is covered.

Docs updated: the distributed-tracing guide's resource-attributes section, the production-tracing checklist, and the configs reference (APP_NAME and OTEL_RESOURCE_ATTRIBUTES rows amended, a new OTEL_SERVICE_NAME row added).

No new dependencies.

Checklist:

• I have formatted my code using goimport and golangci-lint.
• All new code is covered by unit tests.
• This PR does not decrease the overall code coverage.
• I have reviewed the code comments and documentation for clarity.

[aryanmehrotra]

Thanks for the thorough write-up and the mutation table — the SDK claims all check out against the pinned go.opentelemetry.io/otel/sdk v1.46.0 (resource/env.go:40-56,75-88). Reviewed at bb96a4a86.

Before going line by line I would like your opinion on the direction, because my inclination is to keep the v1.62.0 behaviour: APP_NAME is the service name in GoFr, for traces and metrics, and OTEL_SERVICE_NAME / service.name= in OTEL_RESOURCE_ATTRIBUTES does not override it. My reasons:

• It matches how GoFr already treats the standard OTel variables: GoFr-native config wins when both are set (see the comment above metricsExporterConfig in pkg/gofr/container/metrics_exporter.go).
• One name for the application everywhere. With the override, the resource says one thing while the health endpoint, MCP, startup telemetry and the otel_scope_name label say another.
• v1.62.0 shipped the opposite advice one release ago, in a log line and in the tracing guide ("Rename the service with APP_NAME"), and I could not find an issue asking for the change.

I would also not add OTEL_SERVICE_NAME as a fallback for an unset APP_NAME. There was no such fallback before — APP_NAME always resolves, defaulting to gofr-app (pkg/gofr/container/container.go:103) — so it would be new behaviour rather than something to preserve.

Is there a deployment or a user request behind this that I am missing? If there is a concrete case where setting APP_NAME is not workable, that would change the picture, so please say.

If we do keep APP_NAME, this is what I would still like to take from the PR — tell me if you disagree with any of it:

1. The nil-Logger guard in metrics/exporters.Build and its test. It is a real fix: on development, Build(ctx, cfg, nil) with an unknown METRICS_EXPORTER panics on logger.Errorf. One correction to the comments and description: the deprecated Prometheus helper is not a nil-logger caller, it passes noopLogger{} (metrics/exporters/exporter.go:18), so "never applied" is not accurate.

2. Make the ignored value visible on the metrics side too. Your observation is right that traces/exporters.Build returns the NeverSample provider before buildResource when TRACE_EXPORTER is unset, so a metrics-only app drops OTEL_SERVICE_NAME with no log at all. The warning the traces package already has (…from the environment is ignored; GoFr sets it from APP_NAME) would fit in metrics/exporters as well.

3. Docs. The tracing guide already states the rule. The configs reference does not: the APP_NAME row could say it is the service.name on exported traces and metrics, and the OTEL_RESOURCE_ATTRIBUTES row that a service.name= entry is ignored, as is OTEL_SERVICE_NAME.

4. Pin the precedence in metrics/exporters. On development nothing in the metrics suite pins the WithFromEnv / WithAttributes order: swapping the two options passes every metrics test, while the same swap fails two rows of Test_buildResource in traces. Your new metrics Test_buildResource table is the right shape for this with the expectations flipped back (OTEL_SERVICE_NAME loses to APP_NAME and is reported, service.name in OTEL_RESOURCE_ATTRIBUTES loses, its siblings do not), plus the framework_version is not overridable from the environment row in both packages.

One test note either way: with OTEL_SERVICE_NAME exported in the shell, this branch fails TestBuildResource_carriesRequiredLabelSources and TestBuildResource_incompleteDetectionIsLoggedButKeepsAttributes, which both pass on development under the same environment. Table rows that set no environment should clear OTEL_SERVICE_NAME and OTEL_RESOURCE_ATTRIBUTES with t.Setenv(..., "") so they do not depend on the caller's shell.