Skip to content

Add metadata field to FlagDetails and populate allocationKey#2989

Open
typotter wants to merge 11 commits into
developfrom
typo/thread-allocation-key-to-flag-details-metadata
Open

Add metadata field to FlagDetails and populate allocationKey#2989
typotter wants to merge 11 commits into
developfrom
typo/thread-allocation-key-to-flag-details-metadata

Conversation

@typotter

@typotter typotter commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

What and why?

FFL-2510 — FlagDetails lacked a metadata field, making fields from the Precomputed Flags response (like allocationKey and extraLogging primitives) inaccessible to callers who use getDetails. This PR adds metadata: [String: AnyValue] to FlagDetails and populates it from the flag assignment.

How?

  • FlagAssignment.swift: Added extraLogging: [String: AnyValue] (decoded with decodeIfPresent, defaults to [:] for backwards compatibility with older API responses)
  • FlagDetails.swift: Added public var metadata: [String: AnyValue] (default [:], synthesized EquatableAnyValue is already Equatable)
  • FlagsClient.getDetails: Iterates extraLogging primitives into metadata (skipping "allocationKey" key), then writes allocationKey last so the typed field always wins on collision
  • api-surface-swift: Regenerated to include new FlagAssignment.extraLogging and FlagDetails.metadata entries
  • Tests: new testGetDetails_includesAllocationKeyInMetadata, testGetDetails_extraLoggingAllocationKeyDoesNotOverrideTypedAllocationKey, testGetDetails_errorPathsHaveEmptyMetadata, and new decoding tests for extraLogging

Verification

CI passed: Unit Tests (iOS) ✓, Unit Tests (tvOS/watchOS/visionOS) ✓, SPM Build (Swift 6.0/6.2) ✓, Smoke Tests ✓.

Review checklist

  • Feature or bugfix MUST have appropriate tests (unit, integration)
  • Make sure each commit and the PR mention the Issue number or JIRA reference
  • Add CHANGELOG entry for user facing changes
  • Add Objective-C interface for public APIs - see our guidelines (internal)
  • Run make api-surface when adding new APIs

@typotter typotter requested review from a team as code owners June 11, 2026 16:09
@dd-octo-sts-6cbbf8

Copy link
Copy Markdown

🐑 PR Shepherd is maintaining this PR

I watch your PR and automatically fix CI failures, rebase your branch, handle flaky tests, and push it to the merge queue when it's ready.

More about what I do → Guide

To pause me on this PR, add the flow-skip label.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0fab5658b5

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread DatadogFlags/Sources/Models/FlagDetails.swift Outdated
…etadata

- Add `extraLogging: [String: AnyValue]` property to `FlagAssignment` with default `[:]`
- Add `extraLogging` CodingKey and decode with `decodeIfPresent` for backwards compatibility
- Encode `extraLogging` in `encode(to:)`
- In `FlagsClient.getDetails`, merge `extraLogging` primitive values (string, int, double, bool)
  into metadata before writing `allocationKey`; skip any `allocationKey` key in extraLogging
  so the typed field always wins
- Update `testGetDetails_includesAllocationKeyInMetadata` to assert extraLogging values appear in metadata
- Add `testGetDetails_extraLoggingAllocationKeyDoesNotOverrideTypedAllocationKey`
- Update `testDecoding` expected values to include `extraLogging: ["experiment": .bool(true)]`
- Add `testDecodingFlagAssignmentWithExtraLogging` and `testDecodingFlagAssignmentWithoutExtraLogging`
@datadog-prod-us1-5

This comment has been minimized.

@typotter typotter requested a review from sameerank June 11, 2026 17:04
…rror-path metadata tests

- Remove try? from decodeIfPresent so malformed extraLogging payloads surface decode errors
- Skip encoding extraLogging when empty to match round-trip symmetry with decodeIfPresent
- Rename loop variable key to logKey in getDetails to avoid shadowing outer key parameter
- Add testGetDetails_errorPathsHaveEmptyMetadata verifying metadata is empty on all three error paths
@typotter

Copy link
Copy Markdown
Contributor Author

Required by: DataDog/dd-openfeature-provider-swift#23 — the Swift OpenFeature provider depends on FlagDetails.metadata introduced here to populate ProviderEvaluation.flagMetadata. That PR currently pins dd-sdk-ios to this branch; once this PR merges and a release is cut, the provider's Package.swift should be updated to the new version tag.

Comment thread DatadogFlags/Sources/Models/FlagDetails.swift Outdated
Comment thread DatadogFlags/Sources/Client/FlagsClient.swift Outdated
Comment thread DatadogFlags/Sources/Models/FlagAssignment.swift
- Change metadata type from [String: Any] to [String: AnyValue]
- Remove custom == operator; synthesized Equatable now handles [String: AnyValue]
- Remove import Foundation from FlagDetails.swift (no longer needed)
- Update getDetails to assign AnyValue directly instead of switch-casting
- Update tests to use AnyValue enum cases instead of as? casts
- Regenerate api-surface-swift to reflect new metadata field type
@typotter

Copy link
Copy Markdown
Contributor Author

FFL-2510

@typotter typotter requested a review from sameerank June 12, 2026 21:10
sameerank
sameerank previously approved these changes Jun 12, 2026

@sameerank sameerank left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you may also need to add an entry to the "Unreleased" section of CHANGELOG.md

Comment thread api-surface-swift
public var doLog: Bool
public init(allocationKey: String, variationKey: String, variation: Variation, reason: String, doLog: Bool)
public var extraLogging: [String: AnyValue]
public init(allocationKey: String,variationKey: String,variation: Variation,reason: String,doLog: Bool,extraLogging: [String: AnyValue] = [:])

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There used to be spaces between the function arguments and now there aren't. I wonder if there is some configuration mismatch in the make api-surface run that created the pervious version vs yours?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's still an inconsistency in the formatting here by removing the spaces between the arguments

Comment thread DatadogFlags/Sources/Client/FlagsClient.swift Outdated
@typotter typotter requested a review from sameerank June 16, 2026 04:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants