Skip to content

feat: Introduce logging context to Tangle #66

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.

Sign up for GitHub

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

Jump to bottom
Open
morgan-wowk wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat: Introduce logging context to Tangle #66

morgan-wowk wants to merge 1 commit into from
+944 −32

Conversation

@morgan-wowk
Copy link

@morgan-wowk morgan-wowk commented Jan 12, 2026 •
edited
Loading

Issue
#67

Changes:

  • Adds trace context helpers
  • Add trace middleware to apply trace context to API requests
    • This helps logging formatters pull trace ids out of context to set them in logs
    • Sets the trace id on the response for client consumption
  • Wraps API requests with trace context using middleware
  • Passes trace id to pipeline executions using extras parameter
  • Wraps pipeline execution with trace context (using trace id from extras parameter, or creating a new one if None)

Test pipeline notes:

  • Requires ytest, starlette, httpx (e.g. pip3 install ...)

@maxy-shpfy maxy-shpfy requested a review from Ark-kun January 12, 2026 22:55
morgan-wowk
morgan-wowk commented Jan 12, 2026
View reviewed changes
cloud_pipelines_backend/instrumentation/trace_middleware.py Outdated
# Process the request
response = await call_next(request)
# Add trace_id to response headers for client reference
response.headers["x-tangle-trace-id"] = trace_id
Copy link
Author

@morgan-wowk morgan-wowk Jan 12, 2026

Choose a reason for hiding this comment

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

Note to self. This line should probably go above await call_next(request) in case an exception is raised

Ark-kun
Ark-kun reviewed Jan 13, 2026
View reviewed changes
cloud_pipelines_backend/orchestrator_sql.py Outdated
# Restore trace_id from execution
trace_id = None
if queued_execution.extra_data:
trace_id = queued_execution.extra_data.get("trace_id")
Copy link
Contributor

@Ark-kun Ark-kun Jan 13, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I do not think the request trace IDs are relevant when processing execution nodes. Execution nodes already have their own IDs and they might work better as trace IDs.

@Ark-kun Ark-kun self-assigned this Jan 13, 2026
@Ark-kun
Copy link
Contributor

Ark-kun commented Jan 13, 2026

Thank you for this PR.

I'm reviewing the changes and I think we might want to design this slightly differently.
I think that trace IDs are mostly relevant to API Server and API requests.
Orchestrator works autonomously, it's not triggered by API requests, so it does not have them or need them. Instead, the ExecutionNode.id and ContainerExecution.id already serve the same role.

On the other hand, API requests might benefit from trace IDs so that all logging messages that are generated when processing a single API call can be filtered and grouped together.

@morgan-wowk
Copy link
Author

morgan-wowk commented Jan 13, 2026 •
edited
Loading

Thank you for this PR.

I'm reviewing the changes and I think we might want to design this slightly differently. I think that trace IDs are mostly relevant to API Server and API requests. Orchestrator works autonomously, it's not triggered by API requests, so it does not have them or need them. Instead, the ExecutionNode.id and ContainerExecution.id already serve the same role.

On the other hand, API requests might benefit from trace IDs so that all logging messages that are generated when processing a single API call can be filtered and grouped together.

This sounds good to me.

I will make some adjustments. Thanks!

@morgan-wowk morgan-wowk changed the title feat: Introduce trace ids to Tangle feat: Introduce logging context to Tangle Jan 19, 2026
@morgan-wowk
feat: Introduce trace ids to Tangle
987c061 
**Changes:**

* Adds logging context helpers
* Add request middleware to generate unique request id and set it in the logging context around API requests
    * Sets the x-tangle-request-id on the response for client consumption
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Ark-kun Ark-kun Ark-kun left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@Ark-kun Ark-kun

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@morgan-wowk @Ark-kun