Skip to content

Commit eb813a7

Browse files
pengweiqhcaJamesNK
pengweiqhca
and
JamesNK
authored
Fix different activitiy types sent to diagnostic source (#1559)
Co-authored-by: James Newton-King <[email protected]>
1 parent 518a636 commit eb813a7

File tree

5 files changed

+99
-50
lines changed

5 files changed

+99
-50
lines changed

‎src/Grpc.Net.Client/Internal/GrpcCall.NonGeneric.cs‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,5 +182,46 @@ private static StatusCode MapHttpStatusToGrpcCode(HttpStatusCode httpStatusCode)
182182
return StatusCode.Unknown;
183183
}
184184
}
185+
186+
protected internal sealed class ActivityStartData
187+
{
188+
#if NET5_0_OR_GREATER
189+
// Common properties. Properties not in this list could be trimmed.
190+
[DynamicDependency(nameof(HttpRequestMessage.RequestUri), typeof(HttpRequestMessage))]
191+
[DynamicDependency(nameof(HttpRequestMessage.Method), typeof(HttpRequestMessage))]
192+
[DynamicDependency(nameof(Uri.Host), typeof(Uri))]
193+
[DynamicDependency(nameof(Uri.Port), typeof(Uri))]
194+
#endif
195+
internal ActivityStartData(HttpRequestMessage request)
196+
{
197+
Request = request;
198+
}
199+
200+
public HttpRequestMessage Request { get; }
201+
202+
public override string ToString() => $"{{ {nameof(Request)} = {Request} }}";
203+
}
204+
205+
protected internal sealed class ActivityStopData
206+
{
207+
#if NET5_0_OR_GREATER
208+
// Common properties. Properties not in this list could be trimmed.
209+
[DynamicDependency(nameof(HttpRequestMessage.RequestUri), typeof(HttpRequestMessage))]
210+
[DynamicDependency(nameof(HttpRequestMessage.Method), typeof(HttpRequestMessage))]
211+
[DynamicDependency(nameof(Uri.Host), typeof(Uri))]
212+
[DynamicDependency(nameof(Uri.Port), typeof(Uri))]
213+
[DynamicDependency(nameof(HttpResponseMessage.StatusCode), typeof(HttpResponseMessage))]
214+
#endif
215+
internal ActivityStopData(HttpResponseMessage? response, HttpRequestMessage request)
216+
{
217+
Response = response;
218+
Request = request;
219+
}
220+
221+
public HttpResponseMessage? Response { get; }
222+
public HttpRequestMessage Request { get; }
223+
224+
public override string ToString() => $"{{ {nameof(Response)} = {Response}, {nameof(Request)} = {Request} }}";
225+
}
185226
}
186227
}

‎src/Grpc.Net.Client/Internal/GrpcCall.cs‎

Lines changed: 2 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -404,7 +404,7 @@ private void CancelCall(Status status)
404404

405405
internal IDisposable? StartScope()
406406
{
407-
// Only return a scope if the logger is enabled to log
407+
// Only return a scope if the logger is enabled to log
408408
// in at least Critical level for performance
409409
if (Logger.IsEnabled(LogLevel.Critical))
410410
{
@@ -811,7 +811,7 @@ private bool FinishCall(HttpRequestMessage request, bool diagnosticSourceEnabled
811811
if (diagnosticSourceEnabled)
812812
{
813813
// Stop sets the end time if it was unset, but we want it set before we issue the write
814-
// so we do it now.
814+
// so we do it now.
815815
if (activity.Duration == TimeSpan.Zero)
816816
{
817817
activity.SetEndTime(DateTime.UtcNow);
@@ -1058,46 +1058,5 @@ private static void WriteDiagnosticEvent<
10581058
{
10591059
diagnosticSource.Write(name, value);
10601060
}
1061-
1062-
private sealed class ActivityStartData
1063-
{
1064-
#if NET5_0_OR_GREATER
1065-
// Common properties. Properties not in this list could be trimmed.
1066-
[DynamicDependency(nameof(HttpRequestMessage.RequestUri), typeof(HttpRequestMessage))]
1067-
[DynamicDependency(nameof(HttpRequestMessage.Method), typeof(HttpRequestMessage))]
1068-
[DynamicDependency(nameof(Uri.Host), typeof(Uri))]
1069-
[DynamicDependency(nameof(Uri.Port), typeof(Uri))]
1070-
#endif
1071-
internal ActivityStartData(HttpRequestMessage request)
1072-
{
1073-
Request = request;
1074-
}
1075-
1076-
public HttpRequestMessage Request { get; }
1077-
1078-
public override string ToString() => $"{{ {nameof(Request)} = {Request} }}";
1079-
}
1080-
1081-
private sealed class ActivityStopData
1082-
{
1083-
#if NET5_0_OR_GREATER
1084-
// Common properties. Properties not in this list could be trimmed.
1085-
[DynamicDependency(nameof(HttpRequestMessage.RequestUri), typeof(HttpRequestMessage))]
1086-
[DynamicDependency(nameof(HttpRequestMessage.Method), typeof(HttpRequestMessage))]
1087-
[DynamicDependency(nameof(Uri.Host), typeof(Uri))]
1088-
[DynamicDependency(nameof(Uri.Port), typeof(Uri))]
1089-
[DynamicDependency(nameof(HttpResponseMessage.StatusCode), typeof(HttpResponseMessage))]
1090-
#endif
1091-
internal ActivityStopData(HttpResponseMessage? response, HttpRequestMessage request)
1092-
{
1093-
Response = response;
1094-
Request = request;
1095-
}
1096-
1097-
public HttpResponseMessage? Response { get; }
1098-
public HttpRequestMessage Request { get; }
1099-
1100-
public override string ToString() => $"{{ {nameof(Response)} = {Response}, {nameof(Request)} = {Request} }}";
1101-
}
11021061
}
11031062
}

‎test/Grpc.Net.Client.Tests/DiagnosticsTests.cs‎

Lines changed: 47 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
using System.Diagnostics;
2020
using System.Net;
21+
using Google.Protobuf;
2122
using Greet;
2223
using Grpc.Core;
2324
using Grpc.Net.Client.Internal;
@@ -83,20 +84,59 @@ public void DiagnosticListener_MakeCall_ActivityWritten()
8384

8485
var result = new List<KeyValuePair<string, object?>>();
8586

87+
var dataMessageMarshaller = new Marshaller<DataMessage>(m => m.ToByteArray(), data => DataMessage.Parser.ParseFrom(data));
88+
var dataMessageMethod = ClientTestHelpers.GetServiceMethod<DataMessage, DataMessage>(
89+
MethodType.DuplexStreaming,
90+
dataMessageMarshaller,
91+
dataMessageMarshaller);
92+
8693
// Act
94+
HttpRequestMessage? requestMessage1 = null;
95+
HttpResponseMessage? responseMessage1 = null;
96+
HttpRequestMessage? requestMessage2 = null;
97+
HttpResponseMessage? responseMessage2 = null;
98+
8799
using (GrpcDiagnostics.DiagnosticListener.Subscribe(new ObserverToList<KeyValuePair<string, object?>>(result)))
88100
{
89-
var c = invoker.AsyncDuplexStreamingCall<HelloRequest, HelloReply>(ClientTestHelpers.ServiceMethod, string.Empty, new CallOptions());
90-
c.Dispose();
101+
var c1 = invoker.AsyncDuplexStreamingCall<HelloRequest, HelloReply>(ClientTestHelpers.ServiceMethod, string.Empty, new CallOptions());
102+
c1.Dispose();
103+
104+
requestMessage1 = requestMessage;
105+
responseMessage1 = responseMessage;
106+
107+
var c2 = invoker.AsyncDuplexStreamingCall<DataMessage, DataMessage>(dataMessageMethod, string.Empty, new CallOptions());
108+
c2.Dispose();
109+
110+
requestMessage2 = requestMessage;
111+
responseMessage2 = responseMessage;
91112
}
92113

93114
// Assert
94-
Assert.AreEqual(2, result.Count);
115+
Assert.AreEqual(4, result.Count);
116+
117+
// First call
95118
Assert.AreEqual(GrpcDiagnostics.ActivityStartKey, result[0].Key);
96-
Assert.AreEqual(requestMessage, GetValueFromAnonymousType<HttpRequestMessage>(result[0].Value!, "Request"));
119+
Assert.AreEqual(requestMessage1, GetValueFromAnonymousType<HttpRequestMessage>(result[0].Value!, "Request"));
97120
Assert.AreEqual(GrpcDiagnostics.ActivityStopKey, result[1].Key);
98-
Assert.AreEqual(requestMessage, GetValueFromAnonymousType<HttpRequestMessage>(result[1].Value!, "Request"));
99-
Assert.AreEqual(responseMessage, GetValueFromAnonymousType<HttpResponseMessage>(result[1].Value!, "Response"));
121+
Assert.AreEqual(requestMessage1, GetValueFromAnonymousType<HttpRequestMessage>(result[1].Value!, "Request"));
122+
Assert.AreEqual(responseMessage1, GetValueFromAnonymousType<HttpResponseMessage>(result[1].Value!, "Response"));
123+
124+
// Second call
125+
Assert.AreEqual(GrpcDiagnostics.ActivityStartKey, result[2].Key);
126+
Assert.AreEqual(requestMessage2, GetValueFromAnonymousType<HttpRequestMessage>(result[2].Value!, "Request"));
127+
Assert.AreEqual(GrpcDiagnostics.ActivityStopKey, result[3].Key);
128+
Assert.AreEqual(requestMessage2, GetValueFromAnonymousType<HttpRequestMessage>(result[3].Value!, "Request"));
129+
Assert.AreEqual(responseMessage2, GetValueFromAnonymousType<HttpResponseMessage>(result[3].Value!, "Response"));
130+
131+
// Check types are expected
132+
Assert.AreEqual(typeof(GrpcCall.ActivityStartData), result[0].Value!.GetType());
133+
Assert.AreEqual(typeof(GrpcCall.ActivityStopData), result[1].Value!.GetType());
134+
Assert.AreEqual(result[0].Value!.GetType(), result[2].Value!.GetType());
135+
Assert.AreEqual(result[1].Value!.GetType(), result[3].Value!.GetType());
136+
137+
// Check values are unique for each call
138+
Assert.AreNotEqual(result[0].Value, result[2].Value);
139+
Assert.AreNotEqual(result[1].Value, result[3].Value);
100140
}
101141

102142
[Test]
@@ -204,7 +244,7 @@ public void OnNext(T value)
204244

205245
private readonly List<T> _output;
206246
private readonly Predicate<T>? _filter;
207-
private readonly string? _name; // for debugging
247+
private readonly string? _name; // for debugging
208248
#endregion
209249
}
210250
}

‎test/Grpc.Net.Client.Tests/Proto/greet.proto‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,3 +33,7 @@ message HelloRequest {
3333
message HelloReply {
3434
string message = 1;
3535
}
36+
37+
message DataMessage {
38+
bytes data = 1;
39+
}

‎test/Shared/ClientTestHelpers.cs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,11 @@ public static Method<HelloRequest, HelloReply> GetServiceMethod(MethodType? meth
3838
return new Method<HelloRequest, HelloReply>(methodType ?? MethodType.Unary, "ServiceName", "MethodName", requestMarshaller ?? HelloRequestMarshaller, HelloReplyMarshaller);
3939
}
4040

41+
public static Method<TRequest, TResponse> GetServiceMethod<TRequest, TResponse>(MethodType methodType, Marshaller<TRequest> requestMarshaller, Marshaller<TResponse> responseMarshaller)
42+
{
43+
return new Method<TRequest, TResponse>(methodType, "ServiceName", "MethodName", requestMarshaller, responseMarshaller);
44+
}
45+
4146
public static TestHttpMessageHandler CreateTestMessageHandler(HelloReply reply)
4247
{
4348
return TestHttpMessageHandler.Create(async r =>

0 commit comments

Comments
 (0)