enhancement(azure_blob sink): update to latest azure SDK#13453
enhancement(azure_blob sink): update to latest azure SDK#13453spencergilbert merged 4 commits intovectordotdev:masterfrom yvespp:yvespp_update_to_latest_azure_sdk
Conversation
✅ Deploy Preview for vector-project ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
| match request { | ||
| let resp: crate::Result<()> = match response { | ||
| Ok(_) => Ok(()), | ||
| Err(reason) => Err(match reason.downcast_ref::<HttpError>() { |
There was a problem hiding this comment.
You shouldn't need to downcast_ref to check the status code anymore, which should significantly simplify this code.
Check out the example missing_blob.rs that shows how to check the status code specifically.
There was a problem hiding this comment.
Thanks for the input. I tried to change the code but run into a problem I don't know how to solve:
pub fn build_healthcheck(
container_name: String,
client: Arc<ContainerClient>,
) -> crate::Result<Healthcheck> {
let healthcheck = async move {
let response = client.get_properties().into_future().await;
let resp: crate::Result<()> = match response {
Ok(_) => Ok(()),
Err(reason) => match reason.kind() {
ErrorKind::HttpResponse {
status: azure_core::StatusCode::Forbidden,
..
} => Err(Box::new(HealthcheckError::InvalidCredentials)),
ErrorKind::HttpResponse {
status: azure_core::StatusCode::NotFound,
..
} => Err(Box::new(HealthcheckError::UnknownContainer {
container: container_name,
})),
ErrorKind::HttpResponse {
status,
..
} => {
let result = StatusCode::from_u16(status.into());
match result {
Ok(status) => Err(Box::new(HealthcheckError::Unknown { status: status })),
Err(status) => Err("Unknown status code".into()),
}
},
_ => Err(reason.into()),
},
};
resp
};
Ok(healthcheck.boxed())
}The problem is that I can't convert the azure_core::StatusCode into http::StatusCode required for HealthcheckError::Unknown. On the line StatusCode::from_u16(status.into()) I get an error that it can't be converted to u16.
I'm still new to Rust and I'm surprised that this doesn't work as it works with http_types::StatusCode
src/sinks/azure_common/service.rs
Outdated
| let result: Result<PutBlockBlobResponse, Self::Error> = blob | ||
| .into_future() | ||
| .inspect_err(|reason| { | ||
| match reason.downcast_ref::<HttpError>() { |
There was a problem hiding this comment.
Similar to above, you don't need to use downcast_ref here, which will simplify this code.
|
I noticed Vector includes a global retry mechanism that appears to be used for all storage APIs. With the Azure SDK's move to the pipeline architecture, by Azure SDK will retry requests automatically using an exponential backoff policy, starting with 800ms delay, retrying up to 3 times, and up to 60s. |
My guess is Vector wants to manage the retries itself to have more control over it. Maybe someone from the Vector team can comment on how to deal with that? |
This comment was marked as outdated.
This comment was marked as outdated.
Yeah, that's correct, we would like to retain control over the retry behavior to have it consistently implemented for all integrations. We do this, for example, with the AWS SDK by disabling its internal retry behavior. It would be great if we could do that here as well! |
|
The Azure SDK's storage crates do not currently expose a mechanism to disable the retry behavior. Our intent is to add it before the next release. |
tobz
added
the
meta: blocked
|
Sounds like we're blocked then until the Azure SDK exposes the ability to control the retry behavior. |
|
The Azure SDK now provides the ability to disable retry. I've updated this pull request to the latest SDK and it isn't blocked anymore. I had a problem with Another problem was with the time dependency (see Cargo.toml for error). Looks to me like cargo is confused with one- and two-digit patch versions ( |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
spencergilbert
added
ci-condition: integration tests enable
and removed
meta: blocked
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Signed-off-by: Yves Peter <ypwebstuff@gmail.com> Co-authored-by: David Schneider <dsbrng25b@gmail.com>
Signed-off-by: yvespp <ypwebstuff@gmail.com>
|
I rebased 20 minutes ago hopping to fix the datadog integration tests, but they still have the same problem: Now two new tests are also broken (elasticsearch and redis) but they look more like temporary failures. |
Sorry! That's on us, that secret doesn't get included in contributor PRs so those jobs always fail. We're looking for a workaround that avoid it polluting community PRs but haven't been able to implement anything. I'm looking to review this PR this week! |
Soak Test ResultsBaseline: f44a7d5 ExplanationA soak test is an integrated performance test for vector in a repeatable rig, with varying configuration for vector. What follows is a statistical summary of a brief vector run for each configuration across SHAs given above. The goal of these tests are to determine, quickly, if vector performance is changed and to what degree by a pull request. Where appropriate units are scaled per-core. The table below, if present, lists those experiments that have experienced a statistically significant change in their throughput performance between baseline and comparision SHAs, with 90.0% confidence OR have been detected as newly erratic. Negative values mean that baseline is faster, positive comparison. Results that do not exhibit more than a ±8.87% change in mean throughput are discarded. An experiment is erratic if its coefficient of variation is greater than 0.3. The abbreviated table will be omitted if no interesting changes are observed. No interesting changes in throughput with confidence ≥ 90.00% and absolute Δ mean >= ±8.87%: Fine details of change detection per experiment.
|
Signed-off-by: Yves Peter <ypwebstuff@gmail.com>
5 participants
Update the Azure SDK to fix a hang when Azure Blob sink was used with managed identity: Azure/azure-sdk-for-rust#808
There where quite a view changes in the SDK recently which required some code updates.
The conversion from http-types::StatusCode of the SDK to hyper::StatusCode of Vector in the error handling is a bit awkward and maybe could be implemented nicer.