feat: add link to logs #620
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,6 +14,7 @@ | |
|
|
||
|
|
||
| import os | ||
| import requests | ||
| from flask import after_this_request, Blueprint, g, redirect, request, render_template | ||
|
|
||
| from middleware import session | ||
|
|
@@ -101,7 +102,8 @@ def add_header(response): | |
| log(f"Exception when creating a donation: {e}", severity="ERROR") | ||
| return render_template("errors/403.html"), 403 | ||
|
|
||
| trace_id = request.headers.get("X-Cloud-Trace-Context").split("/")[0] | ||
| return redirect(f"/viewDonation?donation_id={donation.id}&trace_id={trace_id}") | ||
|
|
||
|
|
||
| @donations_bp.route("/viewDonation", methods=["GET"]) | ||
|
|
@@ -119,8 +121,24 @@ def webapp_view_donation(): | |
| log(f"Exception when getting a campaign for a donations: {e}", severity="ERROR") | ||
| return render_template("errors/403.html"), 403 | ||
|
|
||
| logs_url = None | ||
| try: | ||
| trace_id = request.args.get("trace_id") | ||
|
There was a problem hiding this comment. Not sure how this gets the trace_id, it's not injected as a query string parameter? You can see how trace is extracted in https://github.com/GoogleCloudPlatform/emblem/blob/main/website/middleware/logging.py There was a problem hiding this comment. See line 106 - the (As explained here, the trace ID from There was a problem hiding this comment. We can talk more broadly about the trace propagation in #625. I'm okay not blocking this, but maybe we should have a TODO pointing to 625 for a more permanent solution? For this specific mode on trace propagation, I don't like the idea that the trace_id is a visible part of the website interface. This may make more sense as session data. @muncus do you have any thoughts on "manual" trace propagation across a web redirect? There was a problem hiding this comment. Is there a specific reason the two requests should share a trace id? My understanding is that traces are intended to model single interactions, and trace IDs are not intended to be reused between requests (hence why you're having a hard time doing so 😄 ). If displaying the donation really must be in the same trace, i'd suggest rendering the template here directly, instead of redirecting. Otherwise i'd let the two actions remain separate. There was a problem hiding this comment. @muncus this is useful context, thanks. We could include both trace IDs in the log filter, perhaps? |
||
|
|
||
| project_req = requests.get( | ||
| "http://metadata.google.internal/computeMetadata/v1/project/project-id", | ||
| headers={"Metadata-Flavor": "Google"}, | ||
| ) | ||
| project_id = project_req.text | ||
|
|
||
| if trace_id and project_id: | ||
| logs_url = f"https://console.cloud.google.com/logs/query;query=trace%3D~%22projects%2F{project_id}%2Ftraces%2F{trace_id}%22?project={project_id}" | ||
|
There was a problem hiding this comment. Do we need to query limit to project when we're already operating within a particular project? There was a problem hiding this comment. Good call-out: TIL you can do cross-project traces. @pattishin do you mind if we drop the per-project filter? |
||
| except Exception as e: | ||
| log(f"Exception when getting trace ID: {e}", severity="WARNING") | ||
|
|
||
| return render_template( | ||
| "donations/view-donation.html", | ||
| donation=donation_instance, | ||
| campaign=campaign_instance, | ||
| logsUrl=logs_url, | ||
| ) | ||
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.
Not the cleanest frontend implementation (it doesn't use Material UI's grid layout for right-alignment), but I'm assuming that's not super-important here. 🙂