-
Notifications
You must be signed in to change notification settings - Fork 140
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
Add PushStream
support to IApiRequest
and support chunked-encoding
#4989
Merged
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
6bf7969
Vendor in PushStreamContent to avoid in-memory buffering
andrewlock 5ed3009
Add support for streaming responses
andrewlock 6cb2e19
Don't require the caller to call close on the stream (as it seems weird)
andrewlock 2eea2b2
Update trimming file
andrewlock a018de5
Add a chunked encoding stream writer
andrewlock 0df770d
Update the built-in HttpClient to use the chunked encoding writer for…
andrewlock 3f76f58
Add support for reading chunked encoded streams to MockHttpParser
andrewlock 679fb6b
Add tests for chunked encoding
andrewlock 7d3b360
Update trimming file
andrewlock File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
16 changes: 16 additions & 0 deletions
16
tracer/src/Datadog.Trace/Agent/Transports/ContentTypeHelper.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
// <copyright file="ContentTypeHelper.cs" company="Datadog"> | ||
// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License. | ||
// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc. | ||
// </copyright> | ||
|
||
#nullable enable | ||
|
||
namespace Datadog.Trace.Agent.Transports; | ||
|
||
internal static class ContentTypeHelper | ||
{ | ||
public static string GetContentType(string contentType, string? multipartBoundary) | ||
=> string.IsNullOrEmpty(multipartBoundary) | ||
? contentType | ||
: $"{contentType}; boundary={multipartBoundary}"; | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
75 changes: 75 additions & 0 deletions
75
tracer/src/Datadog.Trace/Agent/Transports/PushStreamContent.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,75 @@ | ||
// <copyright file="PushStreamContent.cs" company="Datadog"> | ||
// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License. | ||
// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc. | ||
// </copyright> | ||
|
||
// Based on code from https://github.com/aspnet/AspNetWebStack/blob/1231b77d79956152831b75ad7f094f844251b97f/src/System.Net.Http.Formatting/PushStreamContent.cs | ||
// and https://github.com/aspnet/AspNetWebStack/blob/1231b77d79956152831b75ad7f094f844251b97f/src/System.Net.Http.Formatting/Internal/DelegatingStream.cs | ||
// which is licensed as: | ||
// Copyright (c) .NET Foundation. All rights reserved. | ||
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. | ||
|
||
#nullable enable | ||
|
||
#if NETCOREAPP | ||
|
||
using System; | ||
using System.Diagnostics.CodeAnalysis; | ||
using System.IO; | ||
using System.Net; | ||
using System.Net.Http; | ||
using System.Net.Http.Headers; | ||
using System.Threading.Tasks; | ||
|
||
namespace Datadog.Trace.Agent.Transports; | ||
|
||
/// <summary> | ||
/// Provides an <see cref="HttpContent"/> implementation that exposes an output <see cref="Stream"/> | ||
/// which can be written to directly. The ability to push data to the output stream differs from the | ||
/// <see cref="StreamContent"/> where data is pulled and not pushed. | ||
/// </summary> | ||
internal class PushStreamContent : HttpContent | ||
{ | ||
private readonly Func<Stream, Task> _onStreamAvailable; | ||
|
||
/// <summary> | ||
/// Initializes a new instance of the <see cref="PushStreamContent"/> class with the given <see cref="MediaTypeHeaderValue"/>. | ||
/// </summary> | ||
/// <param name="onStreamAvailable">The action to call when an output stream is available. When the | ||
/// output stream is closed or disposed, it will signal to the content that it has completed and the | ||
/// HTTP request or response will be completed.</param> | ||
public PushStreamContent(Func<Stream, Task> onStreamAvailable) | ||
{ | ||
_onStreamAvailable = onStreamAvailable; | ||
} | ||
|
||
/// <summary> | ||
/// When this method is called, it calls the action provided in the constructor with the output | ||
/// stream to write to. The action must not close or dispose the stream. Once the task completes, | ||
/// it will close this content instance and complete the HTTP request or response. | ||
/// </summary> | ||
/// <param name="stream">The <see cref="Stream"/> to which to write.</param> | ||
/// <param name="context">The associated <see cref="TransportContext"/>.</param> | ||
/// <returns>A <see cref="Task"/> instance that is asynchronously serializing the object's content.</returns> | ||
[SuppressMessage("Microsoft.Design", "CA1031:DoNotCatchGeneralExceptionTypes", Justification = "Exception is passed as task result.")] | ||
protected override Task SerializeToStreamAsync(Stream stream, TransportContext? context) | ||
{ | ||
// Note the callee must not close or dispose the stream because they don't own it | ||
// We _could_ use a wrapper stream to enforce that, but then it _requires_ the | ||
// callee to call close/dispose which seems weird | ||
return _onStreamAvailable(stream); | ||
} | ||
|
||
/// <summary> | ||
/// Computes the length of the stream if possible. | ||
/// </summary> | ||
/// <param name="length">The computed length of the stream.</param> | ||
/// <returns><c>true</c> if the length has been computed; otherwise <c>false</c>.</returns> | ||
protected override bool TryComputeLength(out long length) | ||
{ | ||
// We can't know the length of the content being pushed to the output stream. | ||
length = -1; | ||
return false; | ||
} | ||
} | ||
#endif |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Why is this being removed? Also I don't see this file on that path is it supposed to be the
HttpOverStreams\ChunkedEncodingWriteStream.cs
?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.
Oops, looks like it got left over in various rebasing. I'll remove it in a subsequent PR (to save CI). Thanks!