Skip to content

impl(gax-internal): recorder and logging#5148

Merged
coryan merged 4 commits intogoogleapis:mainfrom
coryan:impl-gax-internal-with-client-logging
Mar 26, 2026
Merged

impl(gax-internal): recorder and logging#5148
coryan merged 4 commits intogoogleapis:mainfrom
coryan:impl-gax-internal-with-client-logging

Conversation

@coryan
Copy link
Copy Markdown
Contributor

@coryan coryan commented Mar 26, 2026

Introduce WithLogging a decorator for std::future::Future that creates the L3 logs entries with the future completes.

Part of the work for #4772

Introduce `WithLogging` a decorator for `std::future::Future` that creates the
L3 logs entries with the future completes.
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 26, 2026

Codecov Report

❌ Patch coverage is 98.60465% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 97.90%. Comparing base (6566a1d) to head (43904fa).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...ernal/src/observability/client_signals/recorder.rs 91.30% 2 Missing ⚠️
...bservability/client_signals/with_client_logging.rs 99.47% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5148      +/-   ##
==========================================
+ Coverage   97.89%   97.90%   +0.01%     
==========================================
  Files         212      213       +1     
  Lines       42442    42655     +213     
==========================================
+ Hits        41549    41763     +214     
+ Misses        893      892       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@coryan coryan marked this pull request as ready for review March 26, 2026 01:37
@coryan coryan requested a review from a team as a code owner March 26, 2026 01:37
Comment thread src/gax-internal/src/observability/client_signals/recorder.rs Outdated
Comment thread src/gax-internal/src/observability/client_signals/with_client_logging.rs Outdated
Copy link
Copy Markdown
Contributor

@westarle westarle left a comment

Choose a reason for hiding this comment

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

a bunch of nits...

I think we should fix ERROR -> WARN and allow for sanitizing url.full before we forget.

...and I'd really like to see the 'actionable error' info be included at this log level so people don't have to turn on noisy debug logs to get it. I can imagine this as another PR.

Comment thread src/gax-internal/src/observability/client_signals/recorder.rs Outdated
Comment thread src/gax-internal/src/observability/client_signals/recorder.rs Outdated
Comment thread src/gax-internal/src/observability/client_signals/recorder.rs Outdated
Comment thread src/gax-internal/src/observability/client_signals/recorder.rs Outdated
Comment thread src/gax-internal/src/observability/client_signals/with_client_logging.rs Outdated
Comment thread src/gax-internal/src/observability/client_signals/with_client_logging.rs Outdated
Comment thread src/gax-internal/src/observability/client_signals/with_client_logging.rs Outdated
Copy link
Copy Markdown
Contributor Author

@coryan coryan left a comment

Choose a reason for hiding this comment

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

PTAL

Comment thread src/gax-internal/src/observability/client_signals/recorder.rs Outdated
Comment thread src/gax-internal/src/observability/client_signals/recorder.rs Outdated
Comment thread src/gax-internal/src/observability/client_signals/recorder.rs Outdated
Comment thread src/gax-internal/src/observability/client_signals/recorder.rs Outdated
Comment thread src/gax-internal/src/observability/client_signals/recorder.rs Outdated
{ GCP_CLIENT_REPO } = snapshot.client_repo(),
{ GCP_CLIENT_ARTIFACT } = snapshot.client_artifact(),
{ URL_DOMAIN } = snapshot.default_host(),
{ URL_FULL } = snapshot.url(),
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, added an IOU TODO.

Comment thread src/gax-internal/src/observability/client_signals/with_client_logging.rs Outdated
Comment thread src/gax-internal/src/observability/client_signals/with_client_logging.rs Outdated
Comment thread src/gax-internal/src/observability/client_signals/with_client_logging.rs Outdated
@coryan coryan merged commit 6c28e7c into googleapis:main Mar 26, 2026
36 checks passed
@coryan coryan deleted the impl-gax-internal-with-client-logging branch March 26, 2026 14:08
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.

3 participants