Skip to content

EDU-4084: Update observability.mdx#3415

Open
joe-dakota wants to merge 5 commits into
temporalio:mainfrom
joe-dakota:patch-4
Open

EDU-4084: Update observability.mdx#3415
joe-dakota wants to merge 5 commits into
temporalio:mainfrom
joe-dakota:patch-4

Conversation

@joe-dakota
Copy link
Copy Markdown

@joe-dakota joe-dakota commented Mar 4, 2025

Noting that workflow.UpsertTypedSearchAttributes creates NDE

What does this PR do?

This pull request includes a crucial update to the docs/develop/go/observability.mdx file, adding guidance on handling nondeterminism errors when upserting search attributes in existing workflows.

Notes to reviewers

  • docs/develop/go/observability.mdx: Added a code example and explanation on using versioning to prevent failures caused by nondeterminism errors when upserting search attributes in existing workflows.

@joe-dakota joe-dakota requested a review from a team as a code owner March 4, 2025 15:35
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Mar 4, 2025

CLA assistant check
All committers have signed the CLA.

@fairlydurable fairlydurable added the community-contributor This PR was submitted external to Temporal label Mar 4, 2025
Comment thread docs/develop/go/observability.mdx Outdated
Comment thread docs/develop/go/observability.mdx
@fairlydurable fairlydurable changed the title Update observability.mdx EDU-4084: Update observability.mdx Mar 4, 2025
@joe-dakota joe-dakota requested a review from fairlydurable March 4, 2025 21:31
@joe-dakota
Copy link
Copy Markdown
Author

Ping @fairlydurable ... we hit this issue in production and would like to make sure others see this caveat before repeating the same mishap. 🙏

@fairlydurable
Copy link
Copy Markdown
Contributor

Hi @joe-dakota! Unfortunately the personnel who can perform technical reviews this week are at Replay. Do you have access to any Temporal engineers who can vet this?

@fairlydurable
Copy link
Copy Markdown
Contributor

Thank you for submitting this PR. This issue appears to apply to all SDKs, not just Golang.

My current wording:
Adding Search Attributes modifies your Workflow code.
Because of this, you need to use version markers to avoid non-determinism errors when upserting Search Attributes into existing Workflows.

@joe-dakota
Copy link
Copy Markdown
Author

Thank you for submitting this PR. This issue appears to apply to all SDKs, not just Golang.

My current wording: Adding Search Attributes modifies your Workflow code. Because of this, you need to use version markers to avoid non-determinism errors when upserting Search Attributes into existing Workflows.

Alas, I don't have access to Temporal engineers.

I wasn't quite sure if it applied to golang, or all SDKs. Makes sense that it is applicable to all.

Thanks for the update!

@fairlydurable fairlydurable added waiting-on-tech-review PR is blocked and waiting for tech review from Subject Matter Experts and removed waiting-on-cross-team-tech-review labels Mar 11, 2025
@brianmacdonald-temporal
Copy link
Copy Markdown
Contributor

Update -- see Slack convo here:
https://temporaltechnologies.slack.com/archives/C01FG4BRQVB/p1778859913635519
The community member is right, this pattern produces an NDE.
However, the solution is outdated. Instead, recommend worker versioning.
This change will need to be applied to ALL SDKs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community-contributor This PR was submitted external to Temporal waiting-on-tech-review PR is blocked and waiting for tech review from Subject Matter Experts

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants