Skip to content

Conversation

petzel
Copy link
Contributor

@petzel petzel commented Mar 7, 2025

Motivation and Context

Entity IDs are not being passed through to the assignment logger.

Description

Sets the entity ID on the flag evaluation object.

How has this been tested?

  • Unit test

@petzel petzel requested a review from felipecsl March 7, 2025 06:23
@petzel petzel force-pushed the eric/entity-id-flag-evaluation branch from 4b7e842 to d1b3915 Compare March 7, 2025 06:29
@petzel petzel requested a review from leoromanovsky March 7, 2025 07:22
@petzel
Copy link
Contributor Author

petzel commented Mar 7, 2025

Test failures are caused by ts-ignore usage in tooling in Eppo-exp/sdk-test-data#121

extraLogging: split.extraLogging ?? {},
doLog: allocation.doLog,
flagEvaluationDetails,
entityId: flag.entityId ?? null,
Copy link
Contributor

Choose a reason for hiding this comment

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

🤦🏽

@typotter
Copy link
Collaborator

typotter commented Mar 7, 2025

Test failures are caused by ts-ignore usage in tooling in Eppo-exp/sdk-test-data#121

Similar fix to js-client#182 is required here.

@typotter
Copy link
Collaborator

Test failures are caused by ts-ignore usage in tooling in Eppo-exp/sdk-test-data#121

Similar fix to js-client#182 is required here.

PR/235 sent to fix the test suite.

@petzel petzel force-pushed the eric/entity-id-flag-evaluation branch from f631c24 to 2d513d6 Compare March 11, 2025 06:59
@petzel petzel requested a review from typotter March 11, 2025 07:03
@petzel petzel enabled auto-merge (squash) March 11, 2025 07:10
allocationKey: precomputedFlag.allocationKey ?? '',
extraLogging: precomputedFlag.extraLogging ?? {},
doLog: precomputedFlag.doLog,
entityId: null,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this something that needs to be returned from the assignments endpoint so that the precomputed client has it available as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In order to support automated event logging for the precomputed mode, yes we'll have to pass that down as well.

@petzel petzel merged commit 57f4d72 into main Mar 11, 2025
8 checks passed
@petzel petzel deleted the eric/entity-id-flag-evaluation branch March 11, 2025 08:42
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.

4 participants