-
Notifications
You must be signed in to change notification settings - Fork 10
feat: Add Atlan Attributes to Temporal Runs #880
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📜 Docstring Coverage ReportRESULT: PASSED (minimum: 30.0%, actual: 76.0%) Detailed Coverage Report |
|
🛠 Docs available at: https://k.atlan.dev/application-sdk/LH-88 |
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified FilesNo covered modified files...
|
|
🛠 Full Test Coverage Report: https://k.atlan.dev/coverage/application-sdk/pr/880 |
📦 Trivy Vulnerability Scan Results
Report Summary
Scan Result Details✅ No vulnerabilities found during the scan for |
📦 Trivy Secret Scan Results
Report Summary
Scan Result Details✅ No secrets found during the scan for |
| workflow_args["workflow_run_id"] = get_workflow_run_id() | ||
|
|
||
| # Preserve argo workflow metadata from workflow_config for logging context | ||
| workflow_args["argo_workflow_name"] = workflow_config.get( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we have two types of workflows
1/ cross-over 2.0 = inter-app orchestration (argo) and intra-app orchestration (temporal)
2/ cross-over 1.0 = inter-app orchestration (argo) and intra-app orchestration (argo).
3/ native = inter-app orchestration (mostly temporal) and intra-app orchestration (argo)
Essentially we have two kind of workflows then
1/ inter-app
2/ intra-app
Why not standardise the nomenclature across cross-overs and native so that we do not have to redo with tooling changes. Ideally these names should already be sorted out from product perspective but they aren't. Let this nomenclature be finalised between @anbarasantr and me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will be passing atlan-argo-workflow-id and atlan-argo-workflow-node as input args. In the sdk, we will pickup whichever has atlan-* and propogate the same to logs/otel.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please avoid having any argo references in SDK itself
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry typo, I meant atlan-*, we won't have argo references.
| if LOG_LEVEL == "DEBUG": | ||
| colorize = True | ||
| format_str = atlan_format_str_color | ||
| colorize = LOG_LEVEL == "DEBUG" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wish I had more details but I heard recently there was an issue due to attempt to color logs? I think Nishchith knows more details here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
escape sequences are added to give colour for terminal outputs, this is now behind a flag. I've refactored the same to use a method instead of a string.
If DEBUG flag is enabled, it will show coloured text ( appends some escape chars ), else it's normal text.
| workflow_args["workflow_run_id"] = get_workflow_run_id() | ||
|
|
||
| # Preserve argo workflow metadata from workflow_config for logging context | ||
| workflow_args["argo_workflow_name"] = workflow_config.get( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it shouldn't matter to intra-app workflow if invocation is via argo or temporal inter-app workflow or via an API. Can you help me understand what is the log template that we plan to render using these details
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sample log
2025-12-10 15:24:18 [INFO] application_sdk.interceptors.cleanup atlan-argo-workflow-id=redshift-sai-test-12345 atlan-argo-workflow-node=redshift-sai-test-12345.run(0).extrat(2) - Cleanup completed successfully Workflow Context: Workflow ID: 617170a2-5c89-4c59-b212-03cf10d3d670 Run ID: 019b07a9-c0b4-7d2b-bbbf-01ac5b3a0ea7 Type: RedshiftMetadataExtractionWorkflow
Moved `request_context` and `correlation_context` ContextVar definitions to a new dedicated module to resolve circular import dependencies between `logger_adaptor.py` and `utils.py`. Changes: - Add `application_sdk/observability/context.py` with shared ContextVars - Update `logger_adaptor.py` to import from context.py (re-exports for backward compatibility) - Replace lazy imports with top-level imports in: - `utils.py` - `workflows/__init__.py` - `interceptors/events.py` - `logmiddleware.py` Benefits: - Cleaner dependency graph with explicit imports - No more hidden lazy imports inside functions - Backward compatible - existing imports from logger_adaptor still work - Better code discoverability and maintainability
Modified the way correlation strings are constructed and accessed in the AtlanLoggerAdapter. The correlation string is now directly assigned to the record's extra dictionary, improving clarity and maintainability of the logging format.
… headers and arguments
…nhance logging format - Changed `request_context` and `correlation_context` to allow None as default values. - Added `trace_id` field to `LogExtraModel` for improved logging. - Updated log formatting to include `trace_id` when present, while ensuring atlan-* headers are excluded from the display. - Enhanced tests to verify trace_id extraction and logging behavior.
Signed-off-by: saig214 <[email protected]>
…loads (#878) Signed-off-by: SanilK2108 <[email protected]>
Signed-off-by: saig214 <[email protected]>
📦 Example workflows test results
|
📦 Example workflows test results
|
📦 Example workflows test results
|
Changelog
CorrelationContextInterceptorto automatically propagateatlan-*headers andtrace_idfrom workflows to activities for distributed tracingtrace_idfor request correlationChecklist
Copyleft License Compliance
Note
Adds a Temporal interceptor to carry atlan-* fields and trace_id from workflow args to activity headers, wires it into the worker, and surfaces these in logging/OTEL; includes state/utility updates and tests.
CorrelationContextInterceptorto extract atlan-* andtrace_idfrom workflow args, inject as activity headers, and set activity-side context.interceptorslist.activities/__init__.py:get_workflow_argsnow preserves atlan-* keys fromworkflow_configintoworkflow_args.observability/context.pywithrequest_contextandcorrelation_contextContextVars.logger_adaptor: allow extra fields, include atlan-* extras, displaytrace_idin log format, and merge correlation context (trace_id + atlan-*) into log kwargs; minor cleanups.observability/utils:WorkflowContextallows dynamic atlan-* fields and merges correlation context.request_contextfrom new module.Written by Cursor Bugbot for commit f105d0a. This will update automatically on new commits. Configure here.