From 71735e207e9b910e4d3b371be5cb00dc01e8bd2a Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Tue, 18 Jul 2023 09:41:28 +0800 Subject: [PATCH 1/2] The best gRPC debugging that OSS can buy --- src/Grpc.Core.Api/ClientBase.cs | 30 +++ .../Interceptors/InterceptingCallInvoker.cs | 2 + .../Internal/ClientDebuggerHelpers.cs | 110 ++++++++++ src/Grpc.Core.Api/Method.cs | 2 + src/Grpc.Core.Api/SslCredentials.cs | 2 - src/Grpc.Net.Client/GrpcChannel.cs | 17 ++ .../Internal/HttpClientCallInvoker.cs | 1 + src/Shared/CodeAnalysisAttributes.cs | 80 ++++++++ test/FunctionalTests/Linker/LinkerTests.cs | 193 +++++++++--------- test/Grpc.Core.Api.Tests/ClientTest.cs | 88 ++++++++ .../Grpc.Net.Client.Tests/GrpcChannelTests.cs | 50 +++++ 11 files changed, 479 insertions(+), 96 deletions(-) create mode 100644 src/Grpc.Core.Api/Internal/ClientDebuggerHelpers.cs create mode 100644 test/Grpc.Core.Api.Tests/ClientTest.cs diff --git a/src/Grpc.Core.Api/ClientBase.cs b/src/Grpc.Core.Api/ClientBase.cs index dfc495851..94d230284 100644 --- a/src/Grpc.Core.Api/ClientBase.cs +++ b/src/Grpc.Core.Api/ClientBase.cs @@ -17,6 +17,8 @@ #endregion using System; +using System.Collections.Generic; +using System.Diagnostics; using Grpc.Core.Interceptors; using Grpc.Core.Internal; using Grpc.Core.Utils; @@ -84,6 +86,9 @@ public T WithHost(string host) /// /// Base class for client-side stubs. /// +// The call invoker's debug information is provided by DebuggerDisplayAttribute. It isn't available in ToString. +[DebuggerDisplay("{ServiceNameDebuggerToString(),nq}{CallInvoker}")] +[DebuggerTypeProxy(typeof(ClientBaseDebugType))] public abstract class ClientBase { readonly ClientBaseConfiguration configuration; @@ -141,6 +146,31 @@ internal ClientBaseConfiguration Configuration get { return this.configuration; } } + internal string ServiceNameDebuggerToString() + { + var serviceName = ClientDebuggerHelpers.GetServiceName(GetType()); + if (serviceName == null) + { + return string.Empty; + } + + return $@"Service = ""{serviceName}"", "; + } + + internal sealed class ClientBaseDebugType + { + readonly ClientBase client; + + public ClientBaseDebugType(ClientBase client) + { + this.client = client; + } + + public CallInvoker CallInvoker => client.CallInvoker; + public string? Service => ClientDebuggerHelpers.GetServiceName(client.GetType()); + public List? Methods => ClientDebuggerHelpers.GetServiceMethods(client.GetType()); + } + /// /// Represents configuration of ClientBase. The class itself is visible to /// subclasses, but contents are marked as internal to make the instances opaque. diff --git a/src/Grpc.Core.Api/Interceptors/InterceptingCallInvoker.cs b/src/Grpc.Core.Api/Interceptors/InterceptingCallInvoker.cs index 43bcec5a2..d1e39a77d 100644 --- a/src/Grpc.Core.Api/Interceptors/InterceptingCallInvoker.cs +++ b/src/Grpc.Core.Api/Interceptors/InterceptingCallInvoker.cs @@ -17,6 +17,7 @@ #endregion using System; +using System.Diagnostics; using Grpc.Core.Utils; namespace Grpc.Core.Interceptors; @@ -25,6 +26,7 @@ namespace Grpc.Core.Interceptors; /// Decorates an underlying to /// intercept calls through a given interceptor. /// +[DebuggerDisplay("{invoker}")] internal class InterceptingCallInvoker : CallInvoker { readonly CallInvoker invoker; diff --git a/src/Grpc.Core.Api/Internal/ClientDebuggerHelpers.cs b/src/Grpc.Core.Api/Internal/ClientDebuggerHelpers.cs new file mode 100644 index 000000000..f309fd8ce --- /dev/null +++ b/src/Grpc.Core.Api/Internal/ClientDebuggerHelpers.cs @@ -0,0 +1,110 @@ +#region Copyright notice and license + +// Copyright 2015-2016 gRPC authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#endregion + +using System; +using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; +using System.Reflection; + +namespace Grpc.Core.Internal; + +internal static class ClientDebuggerHelpers +{ +#if NETSTANDARD1_5 + private static TypeInfo? GetParentType(Type clientType) +#else + private static Type? GetParentType(Type clientType) +#endif + { + // Attempt to get the parent type for a generated client. + // A generated client is always nested inside a static type that contains information about the client. + // For example: + // + // public static class Greeter + // { + // private static readonly serviceName = "Greeter"; + // private static readonly Method _sayHelloMethod; + // + // public class GreeterClient { } + // } + + if (!clientType.IsNested) + { + return null; + } + +#if NETSTANDARD1_5 + var parentType = clientType.DeclaringType.GetTypeInfo(); +#else + var parentType = clientType.DeclaringType; +#endif + // Check parent type is static. A C# static type is sealed and abstract. + if (parentType == null || (!parentType.IsSealed && !parentType.IsAbstract)) + { + return null; + } + + return parentType; + } + + [UnconditionalSuppressMessage("Trimmer", "IL2075", Justification = "Only used by debugging. If trimming is enabled then missing data is not displayed in debugger.")] + internal static string? GetServiceName(Type clientType) + { + // Attempt to get the service name from the generated __ServiceName field. + // If the service name can't be resolved then it isn't displayed in the client's debugger display. + var parentType = GetParentType(clientType); + var field = parentType?.GetField("__ServiceName", BindingFlags.Static | BindingFlags.NonPublic); + if (field == null) + { + return null; + } + + return field.GetValue(null)?.ToString(); + } + + [UnconditionalSuppressMessage("Trimmer", "IL2075", Justification = "Only used by debugging. If trimming is enabled then missing data is not displayed in debugger.")] + internal static List? GetServiceMethods(Type clientType) + { + // Attempt to get the service methods from generated method fields. + // If methods can't be resolved then the collection in the client type proxy is null. + var parentType = GetParentType(clientType); + if (parentType == null) + { + return null; + } + + var methods = new List(); + + var fields = parentType.GetFields(BindingFlags.Static | BindingFlags.NonPublic); + foreach (var field in fields) + { + if (IsMethodField(field)) + { + methods.Add((IMethod)field.GetValue(null)); + } + } + return methods; + + static bool IsMethodField(FieldInfo field) => +#if NETSTANDARD1_5 + typeof(IMethod).GetTypeInfo().IsAssignableFrom(field.FieldType); +#else + typeof(IMethod).IsAssignableFrom(field.FieldType); +#endif + } +} diff --git a/src/Grpc.Core.Api/Method.cs b/src/Grpc.Core.Api/Method.cs index a82459f64..7c43e5fb3 100644 --- a/src/Grpc.Core.Api/Method.cs +++ b/src/Grpc.Core.Api/Method.cs @@ -17,6 +17,7 @@ #endregion using System; +using System.Diagnostics; using Grpc.Core.Utils; namespace Grpc.Core; @@ -71,6 +72,7 @@ public interface IMethod /// /// Request message type for this method. /// Response message type for this method. +[DebuggerDisplay("Name = {Name}, ServiceName = {ServiceName}, Type = {Type}")] public class Method : IMethod { readonly MethodType type; diff --git a/src/Grpc.Core.Api/SslCredentials.cs b/src/Grpc.Core.Api/SslCredentials.cs index 0cc4487c6..54a9a87d3 100644 --- a/src/Grpc.Core.Api/SslCredentials.cs +++ b/src/Grpc.Core.Api/SslCredentials.cs @@ -117,5 +117,3 @@ public override void InternalPopulateConfiguration(ChannelCredentialsConfigurato internal override bool IsComposable => true; } - - diff --git a/src/Grpc.Net.Client/GrpcChannel.cs b/src/Grpc.Net.Client/GrpcChannel.cs index a73a45e53..8551523c1 100644 --- a/src/Grpc.Net.Client/GrpcChannel.cs +++ b/src/Grpc.Net.Client/GrpcChannel.cs @@ -38,6 +38,7 @@ namespace Grpc.Net.Client; /// Client objects can reuse the same channel. Creating a channel is an expensive operation compared to invoking /// a remote call so in general you should reuse a single channel for as many calls as possible. /// +[DebuggerDisplay("{DebuggerToString(),nq}")] public sealed class GrpcChannel : ChannelBase, IDisposable { internal const int DefaultMaxReceiveMessageSize = 1024 * 1024 * 4; // 4 MB @@ -845,6 +846,22 @@ public ISubchannelTransport Create(Subchannel subchannel) } #endif + internal string DebuggerToString() + { + var debugText = $@"Address = ""{Address.OriginalString}"""; + if (!IsHttpOrHttpsAddress(Address)) + { + // It is easy to tell whether a channel is secured when the address contains http/https. + // Load balancing use custom schemes. Include IsSecure in debug text for custom schemes. + debugText += $", IsSecure = {(_isSecure ? "true" : "false")}"; + } + if (Disposed) + { + debugText += ", Disposed = true"; + } + return debugText; + } + private readonly struct MethodKey : IEquatable { public MethodKey(string? service, string? method) diff --git a/src/Grpc.Net.Client/Internal/HttpClientCallInvoker.cs b/src/Grpc.Net.Client/Internal/HttpClientCallInvoker.cs index 35712aef4..d6181642d 100644 --- a/src/Grpc.Net.Client/Internal/HttpClientCallInvoker.cs +++ b/src/Grpc.Net.Client/Internal/HttpClientCallInvoker.cs @@ -26,6 +26,7 @@ namespace Grpc.Net.Client.Internal; /// /// A client-side RPC invocation using HttpClient. /// +[DebuggerDisplay("{Channel}")] internal sealed class HttpClientCallInvoker : CallInvoker { internal GrpcChannel Channel { get; } diff --git a/src/Shared/CodeAnalysisAttributes.cs b/src/Shared/CodeAnalysisAttributes.cs index b1e1ac7d6..926966040 100644 --- a/src/Shared/CodeAnalysisAttributes.cs +++ b/src/Shared/CodeAnalysisAttributes.cs @@ -151,4 +151,84 @@ internal enum DynamicallyAccessedMemberTypes All = ~None } +/// +/// Suppresses reporting of a specific rule violation, allowing multiple suppressions on a +/// single code artifact. +/// +/// +/// is different than +/// in that it doesn't have a +/// . So it is always preserved in the compiled assembly. +/// +[AttributeUsage(AttributeTargets.All, Inherited = false, AllowMultiple = true)] +internal sealed class UnconditionalSuppressMessageAttribute : Attribute +{ + /// + /// Initializes a new instance of the + /// class, specifying the category of the tool and the identifier for an analysis rule. + /// + /// The category for the attribute. + /// The identifier of the analysis rule the attribute applies to. + public UnconditionalSuppressMessageAttribute(string category, string checkId) + { + Category = category; + CheckId = checkId; + } + + /// + /// Gets the category identifying the classification of the attribute. + /// + /// + /// The property describes the tool or tool analysis category + /// for which a message suppression attribute applies. + /// + public string Category { get; } + + /// + /// Gets the identifier of the analysis tool rule to be suppressed. + /// + /// + /// Concatenated together, the and + /// properties form a unique check identifier. + /// + public string CheckId { get; } + + /// + /// Gets or sets the scope of the code that is relevant for the attribute. + /// + /// + /// The Scope property is an optional argument that specifies the metadata scope for which + /// the attribute is relevant. + /// + public string? Scope { get; set; } + + /// + /// Gets or sets a fully qualified path that represents the target of the attribute. + /// + /// + /// The property is an optional argument identifying the analysis target + /// of the attribute. An example value is "System.IO.Stream.ctor():System.Void". + /// Because it is fully qualified, it can be long, particularly for targets such as parameters. + /// The analysis tool user interface should be capable of automatically formatting the parameter. + /// + public string? Target { get; set; } + + /// + /// Gets or sets an optional argument expanding on exclusion criteria. + /// + /// + /// The property is an optional argument that specifies additional + /// exclusion where the literal metadata target is not sufficiently precise. For example, + /// the cannot be applied within a method, + /// and it may be desirable to suppress a violation against a statement in the method that will + /// give a rule violation, but not against all statements in the method. + /// + public string? MessageId { get; set; } + + /// + /// Gets or sets the justification for suppressing the code analysis message. + /// + public string? Justification { get; set; } +} + #endif diff --git a/test/FunctionalTests/Linker/LinkerTests.cs b/test/FunctionalTests/Linker/LinkerTests.cs index b7d3b8f14..b69e7a192 100644 --- a/test/FunctionalTests/Linker/LinkerTests.cs +++ b/test/FunctionalTests/Linker/LinkerTests.cs @@ -1,4 +1,4 @@ -#region Copyright notice and license +#region Copyright notice and license // Copyright 2019 The gRPC Authors // @@ -25,126 +25,131 @@ using Grpc.Tests.Shared; using NUnit.Framework; -namespace Grpc.AspNetCore.FunctionalTests.Linker +namespace Grpc.AspNetCore.FunctionalTests.Linker; + +[TestFixture] +[Category("LongRunning")] +public class LinkerTests { - [TestFixture] - [Category("LongRunning")] - public class LinkerTests + private static readonly TimeSpan Timeout = TimeSpan.FromSeconds(120); + + [Test] + public async Task RunWebsiteAndCallWithClient_Success() { - private static readonly TimeSpan Timeout = TimeSpan.FromSeconds(120); + var projectDirectory = typeof(LinkerTests).Assembly + .GetCustomAttributes() + .Single(a => a.Key == "ProjectDirectory") + .Value; + + var tempPath = Path.GetTempPath(); + var linkerTestsClientPath = Path.Combine(tempPath, "LinkerTestsClient"); + var linkerTestsWebsitePath = Path.Combine(tempPath, "LinkerTestsWebsite"); - [Test] - public async Task RunWebsiteAndCallWithClient_Success() + EnsureDeleted(linkerTestsClientPath); + EnsureDeleted(linkerTestsWebsitePath); + + try { - var projectDirectory = typeof(LinkerTests).Assembly - .GetCustomAttributes() - .Single(a => a.Key == "ProjectDirectory") - .Value; + using var cts = new CancellationTokenSource(); + using var websiteProcess = new WebsiteProcess(); + using var clientProcess = new DotNetProcess(); - var tempPath = Path.GetTempPath(); - var linkerTestsClientPath = Path.Combine(tempPath, "LinkerTestsClient"); - var linkerTestsWebsitePath = Path.Combine(tempPath, "LinkerTestsWebsite"); + try + { + var publishWebsiteTask = PublishAppAsync(projectDirectory + @"\..\..\testassets\LinkerTestsWebsite\LinkerTestsWebsite.csproj", linkerTestsWebsitePath, cts.Token); + var publishClientTask = PublishAppAsync(projectDirectory + @"\..\..\testassets\LinkerTestsClient\LinkerTestsClient.csproj", linkerTestsClientPath, cts.Token); - EnsureDeleted(linkerTestsClientPath); - EnsureDeleted(linkerTestsWebsitePath); + await Task.WhenAll(publishWebsiteTask, publishClientTask).TimeoutAfter(Timeout); + Console.WriteLine("Successfully published app."); + } + finally + { + cts.Dispose(); + } try { - using var cts = new CancellationTokenSource(); - using var websiteProcess = new WebsiteProcess(); - using var clientProcess = new DotNetProcess(); - - try - { - var publishWebsiteTask = PublishAppAsync(projectDirectory + @"\..\..\testassets\LinkerTestsWebsite\LinkerTestsWebsite.csproj", linkerTestsWebsitePath, cts.Token); - var publishClientTask = PublishAppAsync(projectDirectory + @"\..\..\testassets\LinkerTestsClient\LinkerTestsClient.csproj", linkerTestsClientPath, cts.Token); - - await Task.WhenAll(publishWebsiteTask, publishClientTask).TimeoutAfter(Timeout); - - websiteProcess.Start(Path.Combine(linkerTestsWebsitePath, "LinkerTestsWebsite.dll")); - await websiteProcess.WaitForReadyAsync().TimeoutAfter(Timeout); - - clientProcess.Start(Path.Combine(linkerTestsClientPath, $"LinkerTestsClient.dll {websiteProcess.ServerPort}")); - await clientProcess.WaitForExitAsync().TimeoutAfter(Timeout); - - Assert.AreEqual(0, clientProcess.ExitCode); - } - finally - { - Console.WriteLine("Website output:"); - Console.WriteLine(websiteProcess.GetOutput()); - Console.WriteLine("Client output:"); - Console.WriteLine(clientProcess.GetOutput()); - - cts.Dispose(); - } + websiteProcess.Start(Path.Combine(linkerTestsWebsitePath, "LinkerTestsWebsite.dll")); + await websiteProcess.WaitForReadyAsync().TimeoutAfter(Timeout); + + clientProcess.Start(Path.Combine(linkerTestsClientPath, $"LinkerTestsClient.dll {websiteProcess.ServerPort}")); + await clientProcess.WaitForExitAsync().TimeoutAfter(Timeout); } finally { - EnsureDeleted(linkerTestsClientPath); - EnsureDeleted(linkerTestsWebsitePath); + Console.WriteLine("Website output:"); + Console.WriteLine(websiteProcess.GetOutput()); + Console.WriteLine("Client output:"); + Console.WriteLine(clientProcess.GetOutput()); } + + Assert.AreEqual(0, clientProcess.ExitCode); + } + finally + { + EnsureDeleted(linkerTestsClientPath); + EnsureDeleted(linkerTestsWebsitePath); } + } - private static void EnsureDeleted(string path) + private static void EnsureDeleted(string path) + { + if (Directory.Exists(path)) { - if (Directory.Exists(path)) - { - Directory.Delete(path, recursive: true); - } + Directory.Delete(path, recursive: true); } + } + + private static async Task PublishAppAsync(string path, string outputPath, CancellationToken cancellationToken) + { + var resolvedPath = Path.GetFullPath(path); + Console.WriteLine($"Publishing {resolvedPath}"); + + var process = new DotNetProcess(); + cancellationToken.Register(() => process.Dispose()); - private static async Task PublishAppAsync(string path, string outputPath, CancellationToken cancellationToken) + try + { + process.Start($"publish {resolvedPath} -r {GetRuntimeIdentifier()} -c Release -o {outputPath} --self-contained"); + await process.WaitForExitAsync().TimeoutAfter(Timeout); + } + catch (Exception ex) + { + throw new Exception("Error while publishing app.", ex); + } + finally { - var resolvedPath = Path.GetFullPath(path); - Console.WriteLine($"Publishing {resolvedPath}"); + var exitCode = process.HasExited ? (int?)process.ExitCode : null; - var process = new DotNetProcess(); - cancellationToken.Register(() => process.Dispose()); + process.Dispose(); + + var output = process.GetOutput(); + Console.WriteLine("Publish output:"); + Console.WriteLine(output); - try - { - process.Start($"publish {resolvedPath} -r {GetRuntimeIdentifier()} -c Release -o {outputPath} --self-contained"); - await process.WaitForExitAsync().TimeoutAfter(Timeout); - } - catch (Exception ex) - { - throw new Exception("Error while publishing app.", ex); - } - finally + if (exitCode != null && exitCode.Value != 0) { - var exitCode = process.HasExited ? (int?)process.ExitCode : null; - - process.Dispose(); - - var output = process.GetOutput(); - Console.WriteLine("Publish output:"); - Console.WriteLine(output); - - if (exitCode != null && exitCode.Value != 0) - { - throw new Exception($"Non-zero exit code returned: {exitCode}"); - } + throw new Exception($"Non-zero exit code returned: {exitCode}"); } } + } - private static string GetRuntimeIdentifier() + private static string GetRuntimeIdentifier() + { + var architecture = RuntimeInformation.OSArchitecture.ToString().ToLower(); + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { - var architecture = RuntimeInformation.OSArchitecture.ToString().ToLower(); - if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) - { - return "win10-" + architecture; - } - if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux)) - { - return "linux-" + architecture; - } - if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) - { - return "osx-" + architecture; - } - throw new InvalidOperationException("Unrecognized operation system platform"); + return "win10-" + architecture; + } + if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux)) + { + return "linux-" + architecture; + } + if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) + { + return "osx-" + architecture; } + throw new InvalidOperationException("Unrecognized operation system platform"); } } diff --git a/test/Grpc.Core.Api.Tests/ClientTest.cs b/test/Grpc.Core.Api.Tests/ClientTest.cs new file mode 100644 index 000000000..f8a508a50 --- /dev/null +++ b/test/Grpc.Core.Api.Tests/ClientTest.cs @@ -0,0 +1,88 @@ +#region Copyright notice and license + +// Copyright 2015 gRPC authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#endregion + +using System; +using Grpc.Core.Internal; +using NUnit.Framework; + +namespace Grpc.Core.Tests; + +public class ClientTest +{ + [Test] + public void NonGeneratedClient() + { + var client = new TestClient(); + + var debugType = new ClientBase.ClientBaseDebugType(client); + + Assert.AreEqual(null, debugType.Service); + Assert.AreEqual(null, debugType.Methods); + } + + [Test] + public void GeneratedClient() + { + var client = new Greeter.GreeterClient(new UnimplementedCallInvoker()); + + var debugType = new ClientBase.ClientBaseDebugType(client); + + Assert.AreEqual("greet.Greeter", debugType.Service); + Assert.AreEqual(1, debugType.Methods!.Count); + Assert.AreEqual("SayHello", debugType.Methods[0].Name); + } + + private class TestClient : ClientBase + { + } + + public static partial class Greeter + { + static readonly string __ServiceName = "greet.Greeter"; + + static readonly Marshaller __Marshaller = Marshallers.Create(_ => Array.Empty(), _ => new object()); + + static readonly Method __Method_SayHello = new Method( + MethodType.Unary, + __ServiceName, + "SayHello", + __Marshaller, + __Marshaller); + + public partial class GreeterClient : ClientBase + { + public GreeterClient(CallInvoker callInvoker) : base(callInvoker) + { + } + + protected GreeterClient(ClientBaseConfiguration configuration) : base(configuration) + { + } + + public virtual AsyncUnaryCall SayHelloAsync(object request, CallOptions options) + { + return CallInvoker.AsyncUnaryCall(__Method_SayHello, null, options, request); + } + + protected override GreeterClient NewInstance(ClientBaseConfiguration configuration) + { + return new GreeterClient(configuration); + } + } + } +} diff --git a/test/Grpc.Net.Client.Tests/GrpcChannelTests.cs b/test/Grpc.Net.Client.Tests/GrpcChannelTests.cs index 26ccd5c8e..9c925c2a6 100644 --- a/test/Grpc.Net.Client.Tests/GrpcChannelTests.cs +++ b/test/Grpc.Net.Client.Tests/GrpcChannelTests.cs @@ -98,6 +98,56 @@ public void Build_SslCredentialsWithHttps_Success() Assert.IsTrue(channel.IsSecure); } + [TestCase("http://localhost")] + [TestCase("https://localhost")] + public void DebuggerToString_HttpAddress_ExpectedResult(string address) + { + // Arrange + var channel = GrpcChannel.ForAddress(address, + CreateGrpcChannelOptions()); + + // Act & Assert + Assert.AreEqual($@"Address = ""{address}""", channel.DebuggerToString()); + } + + [Test] + public void DebuggerToString_Dispose_ExpectedResult() + { + // Arrange + var channel = GrpcChannel.ForAddress("http://localhost", + CreateGrpcChannelOptions()); + channel.Dispose(); + + // Act & Assert + Assert.AreEqual(@"Address = ""http://localhost"", Disposed = true", channel.DebuggerToString()); + } + +#if SUPPORT_LOAD_BALANCING + [Test] + public void DebuggerToString_NonHttpAddress_ExpectedResult() + { + // Arrange + var services = new ServiceCollection(); + services.AddSingleton(); + services.AddSingleton(); + + var handler = new TestHttpMessageHandler(); + var channelOptions = new GrpcChannelOptions + { + ServiceProvider = services.BuildServiceProvider(), + HttpHandler = handler, + Credentials = ChannelCredentials.SecureSsl + }; + var channel = GrpcChannel.ForAddress("test:///localhost", channelOptions); + + // Act + var debugText = channel.DebuggerToString(); + + // Assert + Assert.AreEqual(@"Address = ""test:///localhost"", IsSecure = true", debugText); + } +#endif + [TestCase("http://localhost")] [TestCase("HTTP://localhost")] public void Build_SslCredentialsWithHttp_ThrowsError(string address) From bb5dbb72ccc19c5259ae83c87e9431fdf3c579a8 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Tue, 25 Jul 2023 08:16:50 +0800 Subject: [PATCH 2/2] Apply suggestions from code review --- src/Grpc.Core.Api/ClientBase.cs | 3 ++- src/Grpc.Net.Client/GrpcChannel.cs | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Grpc.Core.Api/ClientBase.cs b/src/Grpc.Core.Api/ClientBase.cs index 94d230284..7379959b9 100644 --- a/src/Grpc.Core.Api/ClientBase.cs +++ b/src/Grpc.Core.Api/ClientBase.cs @@ -86,7 +86,8 @@ public T WithHost(string host) /// /// Base class for client-side stubs. /// -// The call invoker's debug information is provided by DebuggerDisplayAttribute. It isn't available in ToString. +// The call invoker's debug information is specified in DebuggerDisplayAttribute. +// It can't be concatenated inside ServiceNameDebuggerToString() because it isn't available in ToString. [DebuggerDisplay("{ServiceNameDebuggerToString(),nq}{CallInvoker}")] [DebuggerTypeProxy(typeof(ClientBaseDebugType))] public abstract class ClientBase diff --git a/src/Grpc.Net.Client/GrpcChannel.cs b/src/Grpc.Net.Client/GrpcChannel.cs index 8551523c1..da819725e 100644 --- a/src/Grpc.Net.Client/GrpcChannel.cs +++ b/src/Grpc.Net.Client/GrpcChannel.cs @@ -853,6 +853,7 @@ internal string DebuggerToString() { // It is easy to tell whether a channel is secured when the address contains http/https. // Load balancing use custom schemes. Include IsSecure in debug text for custom schemes. + // For example: Address = "dns:///my-dns-server, IsSecure = false debugText += $", IsSecure = {(_isSecure ? "true" : "false")}"; } if (Disposed)