From 111754fd2e907282186f7b1dcfbf173f43afb501 Mon Sep 17 00:00:00 2001 From: Martijn Laarman Date: Fri, 1 Nov 2024 17:56:07 +0100 Subject: [PATCH] Cleanup auditor just a tad --- .../Components/Pipeline/RequestPipeline.cs | 9 ---- .../Configuration/TransportConfiguration.cs | 1 - .../Diagnostics/AuditDiagnosticObserver.cs | 4 +- .../Diagnostics/Auditing/Audit.cs | 9 ++-- .../Auditing/AuditEventExtensions.cs | 43 ----------------- .../Diagnostics/Auditing/Auditable.cs | 45 ----------------- .../Diagnostics/Auditing/Auditor.cs | 48 +++++++++++++++++++ .../Diagnostics/TypedDiagnosticObserver.cs | 34 ++++++------- 8 files changed, 70 insertions(+), 123 deletions(-) delete mode 100644 src/Elastic.Transport/Diagnostics/Auditing/AuditEventExtensions.cs create mode 100644 src/Elastic.Transport/Diagnostics/Auditing/Auditor.cs diff --git a/src/Elastic.Transport/Components/Pipeline/RequestPipeline.cs b/src/Elastic.Transport/Components/Pipeline/RequestPipeline.cs index ac379e9..2fadbc3 100644 --- a/src/Elastic.Transport/Components/Pipeline/RequestPipeline.cs +++ b/src/Elastic.Transport/Components/Pipeline/RequestPipeline.cs @@ -173,9 +173,6 @@ private async ValueTask CallProductEndpointCoreAsync( { using var audit = auditor?.Add(HealthyResponse, _dateTimeProvider, endpoint.Node); - if (audit is not null) - audit.PathAndQuery = endpoint.PathAndQuery; - try { TResponse response; @@ -417,9 +414,6 @@ private async ValueTask PingCoreAsync(bool isAsync, Node node, Auditor? auditor, using var audit = auditor?.Add(PingSuccess, _dateTimeProvider, node); - if (audit is not null) - audit.PathAndQuery = pingEndpoint.PathAndQuery; - TransportResponse response; try @@ -468,9 +462,6 @@ private async ValueTask SniffCoreAsync(bool isAsync, Auditor? auditor, Cancellat var sniffEndpoint = _productRegistration.CreateSniffEndpoint(node, PingAndSniffRequestConfiguration, _settings); using var audit = auditor?.Add(SniffSuccess, _dateTimeProvider, node); - if (audit is not null) - audit.PathAndQuery = sniffEndpoint.PathAndQuery; - Tuple> result; try diff --git a/src/Elastic.Transport/Configuration/TransportConfiguration.cs b/src/Elastic.Transport/Configuration/TransportConfiguration.cs index 72ef3be..adb15db 100644 --- a/src/Elastic.Transport/Configuration/TransportConfiguration.cs +++ b/src/Elastic.Transport/Configuration/TransportConfiguration.cs @@ -124,7 +124,6 @@ public TransportConfiguration(ITransportConfiguration config) // it's important url formatter is repointed to the new instance of ITransportConfiguration UrlFormatter = new UrlFormatter(this); - Accept = config.Accept; AllowedStatusCodes = config.AllowedStatusCodes; Authentication = config.Authentication; diff --git a/src/Elastic.Transport/Diagnostics/AuditDiagnosticObserver.cs b/src/Elastic.Transport/Diagnostics/AuditDiagnosticObserver.cs index 6e99a1d..874f1d6 100644 --- a/src/Elastic.Transport/Diagnostics/AuditDiagnosticObserver.cs +++ b/src/Elastic.Transport/Diagnostics/AuditDiagnosticObserver.cs @@ -14,7 +14,7 @@ internal sealed class AuditDiagnosticObserver : TypedDiagnosticObserver /// public AuditDiagnosticObserver( Action> onNext, - Action onError = null, - Action onCompleted = null + Action? onError = null, + Action? onCompleted = null ) : base(onNext, onError, onCompleted) { } } diff --git a/src/Elastic.Transport/Diagnostics/Auditing/Audit.cs b/src/Elastic.Transport/Diagnostics/Auditing/Audit.cs index 1059016..4c12482 100644 --- a/src/Elastic.Transport/Diagnostics/Auditing/Audit.cs +++ b/src/Elastic.Transport/Diagnostics/Auditing/Audit.cs @@ -26,11 +26,6 @@ internal Audit(AuditEvent type, DateTimeOffset started) /// public Node? Node { get; internal init; } - /// - /// The path of the request. - /// - public string PathAndQuery { get; internal set; } - /// /// The end date and time of the audit. /// @@ -55,6 +50,8 @@ public override string ToString() var tookString = string.Empty; if (took >= TimeSpan.Zero) tookString = $" Took: {took}"; - return Node == null ? $"Event: {Event.GetStringValue()}{tookString}" : $"Event: {Event.GetStringValue()} Node: {Node?.Uri} NodeAlive: {Node?.IsAlive}Took: {tookString}"; + return Node == null + ? $"Event: {Event.GetStringValue()}{tookString}" + : $"Event: {Event.GetStringValue()} Node: {Node?.Uri} NodeAlive: {Node?.IsAlive}Took: {tookString}"; } } diff --git a/src/Elastic.Transport/Diagnostics/Auditing/AuditEventExtensions.cs b/src/Elastic.Transport/Diagnostics/Auditing/AuditEventExtensions.cs deleted file mode 100644 index 43bae54..0000000 --- a/src/Elastic.Transport/Diagnostics/Auditing/AuditEventExtensions.cs +++ /dev/null @@ -1,43 +0,0 @@ -// Licensed to Elasticsearch B.V under one or more agreements. -// Elasticsearch B.V licenses this file to you under the Apache 2.0 License. -// See the LICENSE file in the project root for more information - -using System.Diagnostics; -using Elastic.Transport.Extensions; - -namespace Elastic.Transport.Diagnostics.Auditing; - -internal static class AuditEventExtensions -{ - /// - /// Returns the name of the event to be used for use in . - /// If this return null the event should not be reported on - /// This indicates this event is monitored by a different component already - /// - /// The diagnostic event name representation or null if it should go unreported - public static string GetAuditDiagnosticEventName(this AuditEvent @event) - { - switch(@event) - { - case AuditEvent.SniffFailure: - case AuditEvent.SniffSuccess: - case AuditEvent.PingFailure: - case AuditEvent.PingSuccess: - case AuditEvent.BadResponse: - case AuditEvent.HealthyResponse: - return null; - case AuditEvent.SniffOnStartup: return nameof(AuditEvent.SniffOnStartup); - case AuditEvent.SniffOnFail: return nameof(AuditEvent.SniffOnFail); - case AuditEvent.SniffOnStaleCluster: return nameof(AuditEvent.SniffOnStaleCluster); - case AuditEvent.Resurrection: return nameof(AuditEvent.Resurrection); - case AuditEvent.AllNodesDead: return nameof(AuditEvent.AllNodesDead); - case AuditEvent.MaxTimeoutReached: return nameof(AuditEvent.MaxTimeoutReached); - case AuditEvent.MaxRetriesReached: return nameof(AuditEvent.MaxRetriesReached); - case AuditEvent.BadRequest: return nameof(AuditEvent.BadRequest); - case AuditEvent.NoNodesAttempted: return nameof(AuditEvent.NoNodesAttempted); - case AuditEvent.CancellationRequested: return nameof(AuditEvent.CancellationRequested); - case AuditEvent.FailedOverAllNodes: return nameof(AuditEvent.FailedOverAllNodes); - default: return @event.GetStringValue(); //still cached but uses reflection - } - } -} diff --git a/src/Elastic.Transport/Diagnostics/Auditing/Auditable.cs b/src/Elastic.Transport/Diagnostics/Auditing/Auditable.cs index bd8e757..d002138 100644 --- a/src/Elastic.Transport/Diagnostics/Auditing/Auditable.cs +++ b/src/Elastic.Transport/Diagnostics/Auditing/Auditable.cs @@ -3,49 +3,9 @@ // See the LICENSE file in the project root for more information using System; -using System.Collections; -using System.Collections.Generic; -using Elastic.Transport.Extensions; namespace Elastic.Transport.Diagnostics.Auditing; -/// Collects events -public class Auditor : IReadOnlyCollection -{ - private readonly DateTimeProvider _dateTimeProvider; - private List? _audits; - - internal Auditor(DateTimeProvider dateTimeProvider) => _dateTimeProvider = dateTimeProvider; - - /// - public IEnumerator GetEnumerator() => - _audits?.GetEnumerator() ?? (IEnumerator)new EmptyEnumerator(); - - IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); - - internal Auditable Add(Auditable auditable) - { - _audits ??= new List(); - _audits.Add(auditable.Audit); - return auditable; - } - internal Auditable Add(AuditEvent type, DateTimeProvider dateTimeProvider, Node? node = null) - { - _audits ??= new List(); - var auditable = new Auditable(type, dateTimeProvider, node); - _audits.Add(auditable.Audit); - return auditable; - } - - /// Emits an event that does not need to track a duration - public void Emit(AuditEvent type) => Add(type, _dateTimeProvider).Dispose(); - /// Emits an event that does not need to track a duration - public void Emit(AuditEvent type, Node node) => Add(type, _dateTimeProvider, node).Dispose(); - - /// - public int Count => _audits?.Count ?? 0; -} - internal class Auditable : IDisposable { private readonly Audit _audit; @@ -73,11 +33,6 @@ public Exception Exception set => Audit.Exception = value; } - public string PathAndQuery - { - set => Audit.PathAndQuery = value; - } - public Audit Audit => _audit; public void Dispose() => Audit.Ended = _dateTimeProvider.Now(); diff --git a/src/Elastic.Transport/Diagnostics/Auditing/Auditor.cs b/src/Elastic.Transport/Diagnostics/Auditing/Auditor.cs new file mode 100644 index 0000000..275d258 --- /dev/null +++ b/src/Elastic.Transport/Diagnostics/Auditing/Auditor.cs @@ -0,0 +1,48 @@ +// Licensed to Elasticsearch B.V under one or more agreements. +// Elasticsearch B.V licenses this file to you under the Apache 2.0 License. +// See the LICENSE file in the project root for more information + +using System.Collections; +using System.Collections.Generic; +using Elastic.Transport.Extensions; + +namespace Elastic.Transport.Diagnostics.Auditing; + +/// Collects events +public class Auditor : IReadOnlyCollection +{ + private readonly DateTimeProvider _dateTimeProvider; + private List? _audits; + + internal Auditor(DateTimeProvider dateTimeProvider) => _dateTimeProvider = dateTimeProvider; + + /// + public IEnumerator GetEnumerator() => + _audits?.GetEnumerator() ?? (IEnumerator)new EmptyEnumerator(); + + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); + + internal Auditable Add(Auditable auditable) + { + _audits ??= new List(); + _audits.Add(auditable.Audit); + return auditable; + } + + internal Auditable Add(AuditEvent type, DateTimeProvider dateTimeProvider, Node? node = null) + { + _audits ??= new List(); + var auditable = new Auditable(type, dateTimeProvider, node); + _audits.Add(auditable.Audit); + return auditable; + } + + /// Emits an event that does not need to track a duration + public void Emit(AuditEvent type) => Add(type, _dateTimeProvider).Dispose(); + + /// Emits an event that does not need to track a duration + public void Emit(AuditEvent type, Node node) => Add(type, _dateTimeProvider, node).Dispose(); + + /// + public int Count => _audits?.Count ?? 0; +} diff --git a/src/Elastic.Transport/Diagnostics/TypedDiagnosticObserver.cs b/src/Elastic.Transport/Diagnostics/TypedDiagnosticObserver.cs index cc38b9a..0c8d1b9 100644 --- a/src/Elastic.Transport/Diagnostics/TypedDiagnosticObserver.cs +++ b/src/Elastic.Transport/Diagnostics/TypedDiagnosticObserver.cs @@ -15,17 +15,17 @@ namespace Elastic.Transport.Diagnostics; internal abstract class TypedDiagnosticObserver : IObserver> { private readonly Action> _onNext; - private readonly Action _onError; - private readonly Action _onCompleted; + private readonly Action? _onError; + private readonly Action? _onCompleted; /// protected TypedDiagnosticObserver( Action> onNext, - Action onError = null, - Action onCompleted = null + Action? onError = null, + Action? onCompleted = null ) { - _onNext= onNext; + _onNext= onNext ?? throw new ArgumentNullException(nameof(onNext)); _onError = onError; _onCompleted = onCompleted; } @@ -36,8 +36,8 @@ protected TypedDiagnosticObserver( void IObserver>.OnNext(KeyValuePair value) { - if (value.Value is TOnNext next) _onNext?.Invoke(new KeyValuePair(value.Key, next)); - else if (value.Value == null) _onNext?.Invoke(new KeyValuePair(value.Key, default)); + if (value.Value is TOnNext next) _onNext.Invoke(new KeyValuePair(value.Key, next)); + else if (value.Value == null) _onNext.Invoke(new KeyValuePair(value.Key, default)); else throw new Exception($"{value.Key} received unexpected type {value.Value.GetType()}"); @@ -49,19 +49,19 @@ public abstract class TypedDiagnosticObserver : IObser { private readonly Action> _onNextStart; private readonly Action> _onNextEnd; - private readonly Action _onError; - private readonly Action _onCompleted; + private readonly Action? _onError; + private readonly Action? _onCompleted; /// protected TypedDiagnosticObserver( Action> onNextStart, Action> onNextEnd, - Action onError = null, - Action onCompleted = null + Action? onError = null, + Action? onCompleted = null ) { - _onNextStart = onNextStart; - _onNextEnd = onNextEnd; + _onNextStart = onNextStart ?? throw new ArgumentNullException(nameof(onNextStart)); + _onNextEnd = onNextEnd ?? throw new ArgumentNullException(nameof(onNextEnd)); _onError = onError; _onCompleted = onCompleted; } @@ -72,11 +72,11 @@ protected TypedDiagnosticObserver( void IObserver>.OnNext(KeyValuePair value) { - if (value.Value is TOnNextStart nextStart) _onNextStart?.Invoke(new KeyValuePair(value.Key, nextStart)); - else if (value.Key.EndsWith(".Start") && value.Value is null) _onNextStart?.Invoke(new KeyValuePair(value.Key, default)); + if (value.Value is TOnNextStart nextStart) _onNextStart.Invoke(new KeyValuePair(value.Key, nextStart)); + else if (value.Key.EndsWith(".Start") && value.Value is null) _onNextStart.Invoke(new KeyValuePair(value.Key, default)); - else if (value.Value is TOnNextEnd nextEnd) _onNextEnd?.Invoke(new KeyValuePair(value.Key, nextEnd)); - else if (value.Key.EndsWith(".Stop") && value.Value is null) _onNextEnd?.Invoke(new KeyValuePair(value.Key, default)); + else if (value.Value is TOnNextEnd nextEnd) _onNextEnd.Invoke(new KeyValuePair(value.Key, nextEnd)); + else if (value.Key.EndsWith(".Stop") && value.Value is null) _onNextEnd.Invoke(new KeyValuePair(value.Key, default)); else throw new Exception($"{value.Key} received unexpected type {value.Value.GetType()}");