Skip to content

Conversation

@Gazizonoki
Copy link
Collaborator

Changelog entry

...

Changelog category

  • Not for changelog (changelog entry is not required)

Description for reviewers

...

Copilot AI review requested due to automatic review settings December 5, 2025 19:34
@Gazizonoki Gazizonoki requested a review from a team as a code owner December 5, 2025 19:34
@Gazizonoki Gazizonoki changed the title [C++ SDK] Fixed missing header in stream request [C++ SDK] Fixed missing headers in stream request Dec 5, 2025
@ydbot
Copy link
Collaborator

ydbot commented Dec 5, 2025

Run Extra Tests

Run additional tests for this PR. You can customize:

  • Test Size: small, medium, large (default: all)
  • Test Targets: any directory path (default: ydb/)
  • Sanitizers: ASAN, MSAN, TSAN
  • Coredumps: enable for debugging (default: off)
  • Additional args: custom ya make arguments

▶  Run tests

@github-actions
Copy link

github-actions bot commented Dec 5, 2025

🟢 2025-12-05 19:35:52 UTC The validation of the Pull Request description is successful.

@github-actions
Copy link

github-actions bot commented Dec 5, 2025

2025-12-05 19:37:49 UTC Pre-commit check linux-x86_64-relwithdebinfo for c4d590a has started.
2025-12-05 19:38:06 UTC Artifacts will be uploaded here
2025-12-05 19:40:12 UTC ya make is running...
🟡 2025-12-05 21:43:29 UTC Some tests failed, follow the links below. Going to retry failed tests...

Ya make output | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
41421 38485 0 4 2905 27

2025-12-05 21:43:41 UTC ya make is running... (failed tests rerun, try 2)
🟢 2025-12-05 22:02:44 UTC Tests successful.

Ya make output | Test bloat | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
56 (only retried tests) 38 0 0 0 18

🟢 2025-12-05 22:02:51 UTC Build successful.
🟢 2025-12-05 22:03:14 UTC ydbd size 2.3 GiB changed* by -1.6 MiB, which is <= 0 Bytes vs main: OK

ydbd size dash main: e3ffe79 merge: c4d590a diff diff %
ydbd size 2 466 175 736 Bytes 2 464 481 952 Bytes -1.6 MiB -0.069%
ydbd stripped size 524 847 872 Bytes 524 559 648 Bytes -281.5 KiB -0.055%

*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors authentication and metadata handling for gRPC requests in the C++ SDK. It introduces a new TAuthenticationError exception type to distinguish authentication failures from general contract violations, and consolidates duplicate metadata creation logic into a centralized MakeCallMeta method. The PR also adds support for OpenTelemetry trace context propagation via the TraceParent field.

Key changes:

  • Introduced TAuthenticationError exception class for authentication-specific errors
  • Refactored duplicated metadata creation logic into TGRpcConnectionsImpl::MakeCallMeta()
  • Added TraceParent field to TRpcRequestSettings for OpenTelemetry trace context support

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
ydb/public/sdk/cpp/include/ydb-cpp-sdk/client/types/exceptions/exceptions.h Adds new TAuthenticationError exception class declaration
ydb/public/sdk/cpp/src/client/types/exceptions/exceptions.cpp Implements constructor for TAuthenticationError
ydb/public/sdk/cpp/src/client/impl/internal/rpc_request_settings/settings.h Adds TraceParent field and removes inline header insertion logic
ydb/public/sdk/cpp/src/client/impl/internal/grpc_connections/grpc_connections.h Refactors three methods to use centralized MakeCallMeta() and catch TAuthenticationError (missing required include)
ydb/public/sdk/cpp/src/client/impl/internal/grpc_connections/grpc_connections.cpp Implements MakeCallMeta() method with TraceParent support and updates GetAuthInfo() error handling

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +25 to +26
} catch (const TAuthenticationError& e) {
throw e;
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

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

The catch block for TAuthenticationError at lines 25-26 is redundant. It catches the exception only to immediately rethrow it unchanged. This block can be removed since the exception will naturally propagate without it, simplifying the code.

Suggested change
} catch (const TAuthenticationError& e) {
throw e;

Copilot uses AI. Check for mistakes.
@github-actions
Copy link

github-actions bot commented Dec 5, 2025

2025-12-05 19:38:27 UTC Pre-commit check linux-x86_64-release-asan for c4d590a has started.
2025-12-05 19:38:47 UTC Artifacts will be uploaded here
2025-12-05 19:40:57 UTC ya make is running...
🟡 2025-12-05 21:23:09 UTC Some tests failed, follow the links below. This fail is not in blocking policy yet

Ya make output | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
13384 13178 0 162 37 7

🟢 2025-12-05 21:23:17 UTC Build successful.
🟢 2025-12-05 21:23:48 UTC ydbd size 3.8 GiB changed* by -1.3 MiB, which is <= 0 Bytes vs main: OK

ydbd size dash main: e3ffe79 merge: c4d590a diff diff %
ydbd size 4 127 837 664 Bytes 4 126 457 336 Bytes -1.3 MiB -0.033%
ydbd stripped size 1 532 697 624 Bytes 1 532 274 968 Bytes -412.8 KiB -0.028%

*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation

@Gazizonoki Gazizonoki merged commit 2e149af into ydb-platform:main Dec 8, 2025
19 of 20 checks passed
@ydbot
Copy link
Collaborator

ydbot commented Dec 8, 2025

Backport

To backport this PR, click the button next to the target branch and then click "Run workflow" in the Run Actions UI.

Branch Run
stable-25-2, stable-25-2-1, stable-25-3, stable-25-3-1 ▶  Backport
stable-25-3, stable-25-3-1 ▶  Backport
stable-25-3 ▶  Backport

▶  Backport manual

@Gazizonoki Gazizonoki deleted the fix-missing-headers branch December 8, 2025 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants