Skip to content
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

MonitoringFactory does not trigger callbacks for PutObject and GetObject #3135

Closed
VasylHryhorzhevskyi opened this issue Oct 2, 2024 · 6 comments
Assignees
Labels
bug This issue is a bug. p2 This is a standard priority issue

Comments

@VasylHryhorzhevskyi
Copy link

Describe the bug

Used C++ SDK version 1.11.407
During the application working there was calls fro requests:
HeadBucket
CreateSession
CreateBucket
ListObjectsV2
DeleteObjects

But for previous version of SDK there was triggered also:
PutObject
GetObject

Also we have moved from cUrl and WinHttp to S3Crt and CrtHttp.

Do you need something else?

Expected Behavior

MonitoringFactory triggers:
PutObject
GetObject

Current Behavior

MonitoringFactory does not trigger:
PutObject
GetObject

Reproduction Steps

  1. use MonitoringFactory for the application
  2. do PutObject or GetObject
  3. check requestName in the MonitoringFactory::OnRequestFailed or MonitoringFactory::OnRequestSucceeded functions calls.

Currently step 3 does not happens.

Possible Solution

No response

Additional Information/Context

Application build for Mac Win and Al2 operations systems.

AWS CPP SDK version used

1.11.407

Compiler and Version used

clang-1500.3.9.4,

Operating System and version

Windows_Server-2022-English-Full-Base-2024.06.13, amzn-ec2-macos-14.6.1-20240814-230108, Amazon Linux 2 AMI (HVM) - Kernel 5.10

@VasylHryhorzhevskyi VasylHryhorzhevskyi added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Oct 2, 2024
@sbera87
Copy link
Contributor

sbera87 commented Oct 3, 2024

On checking the code, PutObject and GetObject for crt client doesn't invoke the monitoring callback methods. While we explore how to add support for this, please also consider using telemetry provider already available in client configuration.

https://github.com/aws/aws-sdk-cpp/blob/main/src/aws-cpp-sdk-core/include/aws/core/client/ClientConfiguration.h#L417-L421

@jmklix jmklix added p2 This is a standard priority issue and removed needs-triage This issue or PR still needs to be triaged. labels Oct 3, 2024
@sbera87 sbera87 self-assigned this Oct 4, 2024
@sbera87
Copy link
Contributor

sbera87 commented Oct 5, 2024

On checking the code, PutObject and GetObject for crt client doesn't invoke the monitoring callback methods. While we explore how to add support for this, please also consider using telemetry provider already available in client configuration.

https://github.com/aws/aws-sdk-cpp/blob/main/src/aws-cpp-sdk-core/include/aws/core/client/ClientConfiguration.h#L417-L421

Monitor support for s3 crt client get/put object is underway.

@VasylHryhorzhevskyi
Copy link
Author

VasylHryhorzhevskyi commented Oct 7, 2024

We use AWS monitoring to get the following information:

  1. Status of individual request (re)try (success/failure)
  2. Thread ID (or similar information about context to distinguish interleaved log messages from requests that complete in parallel)
  3. Request name (PutObject, GetObject, HeadBucket, etc)
  4. Number of attempts after which request succeeded or failed
  5. HTTP response code
  6. Remote IP
  7. Whether request is retry-able (see AWSError::ShouldRetry())
  8. AWSError::GetErrorType()
  9. AWSError::GetExceptionName()
  10. AWSError::GetMessage()
  11. AWS request ID (see AWSError::GetRequestId())
  12. Duration of individual request (re)try and total duration of request (all retries added up)
  13. HTTP client metrics (connect latency, ssl latency, etc)
  14. Request headers
  15. Response headers
  16. Response XML payload
  17. Request URI (for PutObject and GetObject it includes object name)

@sbera87 could you provide a sample code for telemetry provider that demonstrates how to obtain this information?

@sbera87
Copy link
Contributor

sbera87 commented Oct 24, 2024

So, telemetry provides tracer (for logging data points) and a meter (for statistics on data points).
You need to use this with the the utils:

Span: src/aws-cpp-sdk-core/include/smithy/tracing/TraceSpan.h

Meter: src/aws-cpp-sdk-core/include/smithy/tracing/Meter.h

Example usage in sdk:
Constructor for telemetry provider:

m_telemetryProvider(configuration.telemetryProvider ? configuration.telemetryProvider : configuration.configFactories.telemetryProviderCreateFn()),

Usage of meter:

auto meter = m_clientConfiguration.telemetryProvider->getMeter(this->GetServiceClientName(), {});

How sdk measures http metrics:

TracingUtils::EmitCoreHttpMetrics(httpRequest->GetRequestMetrics(),

It does mean that with telemetry provider you need to implement those hooks that are called in the sdk

@sbera87
Copy link
Contributor

sbera87 commented Oct 24, 2024

Monitoring has been added for the concerned S3 APIs. Will close this for now . If any issue faced, please reopen this or create a new issue.

@sbera87 sbera87 closed this as completed Oct 24, 2024
Copy link

This issue is now closed. Comments on closed issues are hard for our team to see.
If you need more assistance, please open a new issue that references this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. p2 This is a standard priority issue
Projects
None yet
Development

No branches or pull requests

3 participants