feat: add setting to use semconv metrics name

[sylr]

No description provided.

[coderabbitai]

Walkthrough

Added a per-deployment boolean setting semconv-metrics-names and a deployment mutator that reads it and, when true, injects SEMCONV_METRICS_NAME=true into all init and regular containers. Documentation, unit tests, and test mocks were added.

Changes

Cohort / File(s)	Summary
Documentation docs/09-Configuration reference/01-Settings.md	Added a settings row for deployments.<deployment-name>.semconv-metrics-names (Bool, default true) describing that SEMCONV_METRICS_NAME=true will be injected into containers when enabled
Implementation internal/resources/applications/application.go	Added withSemconvMetricsNames(ctx) core.ObjectMutator[*appsv1.Deployment] which reads the per-deployment setting and injects SEMCONV_METRICS_NAME=true into init and regular containers; integrated this mutator into the deployment mutator chain in handleDeployment
Tests — behavior internal/resources/applications/application_test.go	Added TestWithSemconvMetricsNames covering enabled/disabled cases, existing env vars, and multiple containers; replaced large mock scaffolding with focused test path and a minimal mockContext
Tests — mocks internal/resources/applications/mock_test.go	Introduced mockManager and mockContext types with minimal, no-op implementations to satisfy controller-runtime interfaces for unit tests

Sequence Diagram

sequenceDiagram
    participant Handler as Deployment Handler
    participant Chain as Mutator Chain
    participant Mutator as withSemconvMetricsNames
    participant Store as Setting Store
    participant Deployment as Deployment Object

    Handler->>Chain: run mutators (including withSemconvMetricsNames)
    Chain->>Mutator: invoke
    Mutator->>Store: read deployments.<name>.semconv-metrics-names
    Store-->>Mutator: boolean value
    alt enabled
        Mutator->>Deployment: inject SEMCONV_METRICS_NAME=true into init containers
        Mutator->>Deployment: inject SEMCONV_METRICS_NAME=true into regular containers
    end
    Mutator-->>Chain: return mutated deployment

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

• Pay attention to env var injection logic to avoid duplicating existing SEMCONV_METRICS_NAME entries and to preserve existing env ordering.
• Review mutator integration in handleDeployment for correct chaining and error propagation.
• Inspect new test mocks (mockManager, mockContext) for interface completeness and potential gaps when used in other tests.

Poem

🐇 I nudged a flag into each pod’s seams,

SEMCONV_METRICS true — small, bright as beams.
Init and main, a tidy little line,
Observability hums, metrics read fine.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.
Description check	❓ Inconclusive	No pull request description was provided by the author; the description field is empty.	Add a description explaining the purpose, rationale, and impact of enabling the semconv metrics name setting.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: adding a new setting to enable semantic convention metrics names across the changeset.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch feat/semconv-metrics-name

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

internal/resources/applications/application_test.go (1)

206-283: Consider calling the actual function for more robust unit testing.

The test currently simulates the behavior manually rather than calling app.withSemconvMetricsNames(ctx)(deployment). While the comments indicate integration tests cover the full behavior, the unit test would be more valuable if it actually invoked the function being tested.

Compare with TestWithNodeIP (line 171), which calls app.withNodeIP(nil)(tc.deployment) directly. You could follow a similar pattern here, though you'd need to handle the context/settings dependency (perhaps with a mock or by accepting that the setting defaults to false).

The current test does verify the env var injection logic correctly, so this is a nice-to-have improvement rather than a critical issue.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 0651b89 and 7ac674f.

📒 Files selected for processing (3)

• docs/09-Configuration reference/01-Settings.md (1 hunks)
• internal/resources/applications/application.go (2 hunks)
• internal/resources/applications/application_test.go (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (2)

internal/resources/applications/application_test.go (1)

internal/core/env.go (1)

• Env (11-16)

internal/resources/applications/application.go (2)

internal/core/utils.go (1)

• ObjectMutator (70-70)

internal/resources/settings/helpers.go (1)

• GetBoolOrFalse (244-246)

🔇 Additional comments (3)

docs/09-Configuration reference/01-Settings.md (1)

43-43: LGTM! Clear documentation for the new setting.

The documentation entry is well-formatted and clearly describes the behavior of the new semconv-metrics-names setting. The description accurately reflects that SEMCONV_METRICS_NAME will be injected into all containers when enabled.

internal/resources/applications/application.go (2)

383-408: LGTM! Well-implemented mutator following established patterns.

The withSemconvMetricsNames method correctly:

• Retrieves the per-deployment setting using the documented key structure
• Returns early when the setting is disabled (opt-in behavior)
• Injects SEMCONV_METRICS_NAME=true into both init containers and regular containers
• Follows the same pattern as withJsonLogging for consistency

---

456-456: LGTM! Mutator correctly added to the chain.

The new mutator is properly integrated into the deployment mutator chain alongside other environment variable mutators.

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between b038fd8 and 663cdf0.

📒 Files selected for processing (4)

• docs/09-Configuration reference/01-Settings.md (1 hunks)
• internal/resources/applications/application.go (2 hunks)
• internal/resources/applications/application_test.go (2 hunks)
• internal/resources/applications/mock_test.go (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• internal/resources/applications/application.go

🧰 Additional context used

🧬 Code graph analysis (2)

internal/resources/applications/application_test.go (6)

api/formance.com/v1beta1/settings_types.go (2)

• Settings (208-213)
• SettingsSpec (23-31)

internal/resources/settings/helpers.go (1)

• SplitKeywordWithDot (416-435)

internal/core/context.go (1)

• Context (10-16)

internal/core/platform.go (1)

• Platform (3-13)

api/formance.com/v1beta1/ledger_types.go (2)

• Ledger (47-53)
• LedgerSpec (23-28)

internal/resources/applications/application.go (1)

• Application (125-130)

internal/resources/applications/mock_test.go (3)

internal/tests/internal/bootstrap.go (1)

• GetScheme (43-45)

internal/core/platform.go (1)

• Platform (3-13)

internal/core/context.go (1)

• Context (10-16)