diff --git a/csharp/src/Apache.Arrow.Adbc/Tracing/ActivityExtensions.cs b/csharp/src/Apache.Arrow.Adbc/Tracing/ActivityExtensions.cs index 1d5a0beb1e..3ab58e9352 100644 --- a/csharp/src/Apache.Arrow.Adbc/Tracing/ActivityExtensions.cs +++ b/csharp/src/Apache.Arrow.Adbc/Tracing/ActivityExtensions.cs @@ -30,6 +30,7 @@ public static class ActivityExtensions /// The name of the event. /// The optional list of tags to attach to the event. /// for convenient chaining. + // TODO: [Obsolete("This method is deprecated. Please use the methods from ActivityWithPii, instead.")] public static Activity? AddEvent(this Activity? activity, string eventName, IReadOnlyList>? tags = default) { if (activity == null) return activity; @@ -43,6 +44,7 @@ public static class ActivityExtensions /// The traceParent id for the associated . /// The optional list of tags to attach to the event. /// for convenient chaining. + // TODO: [Obsolete("This method is deprecated. Please use the methods from ActivityWithPii, instead.")] public static Activity? AddLink(this Activity? activity, string traceParent, IReadOnlyList>? tags = default) { if (activity == null) return activity; @@ -58,6 +60,7 @@ public static class ActivityExtensions /// for convenient chaining. /// The tag key name as a function /// The tag value mapped to the input key as a function + // TODO: [Obsolete("This method is deprecated. Please use the methods from ActivityWithPii, instead.")] public static Activity? AddTag(this Activity? activity, string key, Func value) { return activity?.AddTag(key, value()); @@ -72,6 +75,7 @@ public static class ActivityExtensions /// The tag key name as a function /// The tag value mapped to the input key /// /// The condition to check before adding the tag + // TODO: [Obsolete("This method is deprecated. Please use the methods from ActivityWithPii, instead.")] public static Activity? AddConditionalTag(this Activity? activity, string key, string? value, bool condition) { if (condition) @@ -91,6 +95,7 @@ public static class ActivityExtensions /// The tag key name /// The tag value mapped to the input key as a function /// The format indicator for 16-byte GUID arrays. + // TODO: [Obsolete("This method is deprecated. Please use the methods from ActivityWithPii, instead.")] public static Activity? AddTag(this Activity? activity, string key, byte[]? value, string? guidFormat) { if (value == null) diff --git a/csharp/src/Apache.Arrow.Adbc/Tracing/ActivityTrace.cs b/csharp/src/Apache.Arrow.Adbc/Tracing/ActivityTrace.cs index b332590a56..bd2f7991fd 100644 --- a/csharp/src/Apache.Arrow.Adbc/Tracing/ActivityTrace.cs +++ b/csharp/src/Apache.Arrow.Adbc/Tracing/ActivityTrace.cs @@ -62,6 +62,8 @@ public ActivityTrace(string? activitySourceName = default, string? activitySourc /// public string ActivitySourceName => ActivitySource.Name; + #region Obsolete methods + /// /// Invokes the delegate within the context of a new started . /// @@ -75,6 +77,7 @@ public ActivityTrace(string? activitySourceName = default, string? activitySourc /// status is set to and an Activity is added to the activity /// and finally the exception is rethrown. /// + // TODO: [Obsolete("This method is deprecated. Please use TraceActivity overloads that take ActivityWithPii instead of Activity to avoid accidentally logging PII data.")] public void TraceActivity(Action call, [CallerMemberName] string? activityName = default, string? traceParent = default) { using Activity? activity = StartActivityInternal(activityName, ActivitySource, traceParent ?? TraceParent); @@ -104,6 +107,7 @@ public void TraceActivity(Action call, [CallerMemberName] string? act /// If an exception is thrown by the delegate, the Activity status is set to /// and an Event is added to the activity and finally the exception is rethrown. /// + // TODO: [Obsolete("This method is deprecated. Please use TraceActivity overloads that take ActivityWithPii instead of Activity to avoid accidentally logging PII data.")] public T TraceActivity(Func call, [CallerMemberName] string? activityName = default, string? traceParent = default) { using Activity? activity = StartActivityInternal(activityName, ActivitySource, traceParent ?? TraceParent); @@ -133,6 +137,7 @@ public T TraceActivity(Func call, [CallerMemberName] string? ac /// If an exception is thrown by the delegate, the Activity status is set to /// and an Event is added to the activity and finally the exception is rethrown. /// + // TODO: [Obsolete("This method is deprecated. Please use TraceActivity overloads that take ActivityWithPii instead of Activity to avoid accidentally logging PII data.")] public async Task TraceActivityAsync(Func call, [CallerMemberName] string? activityName = default, string? traceParent = default) { using Activity? activity = StartActivityInternal(activityName, ActivitySource, traceParent ?? TraceParent); @@ -162,6 +167,7 @@ public async Task TraceActivityAsync(Func call, [CallerMemberNa /// If an exception is thrown by the delegate, the Activity status is set to /// and an Event is added to the activity and finally the exception is rethrown. /// + // TODO: [Obsolete("This method is deprecated. Please use TraceActivity overloads that take ActivityWithPii instead of Activity to avoid accidentally logging PII data.")] public async Task TraceActivityAsync(Func> call, [CallerMemberName] string? activityName = default, string? traceParent = default) { using Activity? activity = StartActivityInternal(activityName, ActivitySource, traceParent ?? TraceParent); @@ -192,6 +198,7 @@ public async Task TraceActivityAsync(Func> call, [Calle /// If an exception is thrown by the delegate, the Activity status is set to /// and an Event is added to the activity and finally the exception is rethrown. /// + // TODO: [Obsolete("This method is deprecated. Please use TraceActivity overloads that take ActivityWithPii instead of Activity to avoid accidentally logging PII data.")] public static async Task TraceActivityAsync(ActivitySource activitySource, Func call, [CallerMemberName] string? activityName = default, string? traceParent = default) { using Activity? activity = StartActivityInternal(activityName, activitySource, traceParent); @@ -222,6 +229,7 @@ public static async Task TraceActivityAsync(ActivitySource activitySource, Func< /// If an exception is thrown by the delegate, the Activity status is set to /// and an Event is added to the activity and finally the exception is rethrown. /// + // TODO: [Obsolete("This method is deprecated. Please use TraceActivity overloads that take ActivityWithPii instead of Activity to avoid accidentally logging PII data.")] public static async Task TraceActivityAsync(ActivitySource activitySource, Func> call, [CallerMemberName] string? activityName = default, string? traceParent = default) { using Activity? activity = StartActivityInternal(activityName, activitySource, traceParent); @@ -238,6 +246,197 @@ public static async Task TraceActivityAsync(ActivitySource activitySource, } } + #endregion + + /// + /// Invokes the delegate within the context of a new started . + /// + /// The name of the activity to start. + /// The delegate to call within the context of a newly started + /// The name of the method for the activity. + /// The trace parent context, which is used to link the activity to a distributed trace. + /// An indicator that should be set to true if the exception contains PII data and should be treated accordingly when added to the activity. + /// Returns a new object if there is any listener to the Activity, returns null otherwise + /// + /// Creates and starts a new object if there is any listener for the ActivitySource. + /// Passes the Activity to the delegate and invokes the delegate. If there are no exceptions thrown by the delegate the + /// Activity status is set to . If an exception is thrown by the delegate, the Activity + /// status is set to and an Activity is added to the activity + /// and finally the exception is rethrown. + /// + public void TraceActivity(string activityName, Action call, string? traceParent = default, bool exceptionHasPii = true) + { + using ActivityWithPii? activity = StartActivityWithPiiInternal(activityName, ActivitySource, traceParent ?? TraceParent); + try + { + call(activity); + if (activity?.Status == ActivityStatusCode.Unset) activity?.SetStatus(ActivityStatusCode.Ok); + } + catch (Exception ex) + { + TraceExceptionWithPii(ex, activity, exceptionHasPii); + throw; + } + } + + /// + /// Invokes the delegate within the context of a new started . + /// + /// The return type for the delegate. + /// The name of the activity to start. + /// The delegate to call within the context of a newly started + /// The trace parent context, which is used to link the activity to a distributed trace. + /// An indicator that should be set to true if the exception contains PII data and should be treated accordingly when added to the activity. + /// The result of the call to the delegate. + /// + /// Creates and starts a new object if there is any listener for the ActivitySource. + /// Passes the Activity to the delegate and invokes the delegate. If there are no exceptions thrown by the delegate the + /// Activity status is set to and the result is returned. + /// If an exception is thrown by the delegate, the Activity status is set to + /// and an Event is added to the activity and finally the exception is rethrown. + /// + public T TraceActivity(string activityName, Func call, string? traceParent = default, bool exceptionHasPii = true) + { + using ActivityWithPii? activity = StartActivityWithPiiInternal(activityName, ActivitySource, traceParent ?? TraceParent); + try + { + T? result = call(activity); + if (activity?.Status == ActivityStatusCode.Unset) activity?.SetStatus(ActivityStatusCode.Ok); + return result; + } + catch (Exception ex) + { + TraceExceptionWithPii(ex, activity, exceptionHasPii); + throw; + } + } + + /// + /// Invokes the delegate within the context of a new started . + /// + /// The name of the activity to start. + /// The delegate to call within the context of a newly started + /// The trace parent context, which is used to link the activity to a distributed trace. + /// An indicator that should be set to true if the exception contains PII data and should be treated accordingly when added to the activity. + /// + /// + /// Creates and starts a new object if there is any listener for the ActivitySource. + /// Passes the Activity to the delegate and invokes the delegate. If there are no exceptions thrown by the delegate the + /// Activity status is set to and the result is returned. + /// If an exception is thrown by the delegate, the Activity status is set to + /// and an Event is added to the activity and finally the exception is rethrown. + /// + public async Task TraceActivityAsync(string activityName, Func call, string? traceParent = default, bool exceptionHasPii = false) + { + using ActivityWithPii? activity = StartActivityWithPiiInternal(activityName, ActivitySource, traceParent ?? TraceParent); + try + { + await call(activity); + if (activity?.Status == ActivityStatusCode.Unset) activity?.SetStatus(ActivityStatusCode.Ok); + } + catch (Exception ex) + { + TraceExceptionWithPii(ex, activity, exceptionHasPii); + throw; + } + } + + /// + /// Invokes the delegate within the context of a new started . + /// + /// The return type for the delegate. + /// The name of the activity to start. + /// The delegate to call within the context of a newly started + /// The trace parent context, which is used to link the activity to a distributed trace. + /// An indicator that should be set to true if the exception contains PII data and should be treated accordingly when added to the activity. + /// The result of the call to the delegate. + /// + /// Creates and starts a new object if there is any listener for the ActivitySource. + /// Passes the Activity to the delegate and invokes the delegate. If there are no exceptions thrown by the delegate the + /// Activity status is set to and the result is returned. + /// If an exception is thrown by the delegate, the Activity status is set to + /// and an Event is added to the activity and finally the exception is rethrown. + /// + public async Task TraceActivityAsync(string activityName, Func> call, string? traceParent = default, bool exceptionHasPii = true) + { + using ActivityWithPii? activity = StartActivityWithPiiInternal(activityName, ActivitySource, traceParent ?? TraceParent); + try + { + T? result = await call(activity); + if (activity?.Status == ActivityStatusCode.Unset) activity?.SetStatus(ActivityStatusCode.Ok); + return result; + } + catch (Exception ex) + { + TraceExceptionWithPii(ex, activity, exceptionHasPii); + throw; + } + } + + /// + /// Invokes the delegate within the context of a new started . + /// + /// The to start the on. + /// The name of the activity to start. + /// The delegate to call within the context of a newly started + /// The trace parent context, which is used to link the activity to a distributed trace. + /// An indicator that should be set to true if the exception contains PII data and should be treated accordingly when added to the activity. + /// + /// + /// Creates and starts a new object if there is any listener for the ActivitySource. + /// Passes the Activity to the delegate and invokes the delegate. If there are no exceptions thrown by the delegate the + /// Activity status is set to and the result is returned. + /// If an exception is thrown by the delegate, the Activity status is set to + /// and an Event is added to the activity and finally the exception is rethrown. + /// + public static async Task TraceActivityAsync(ActivitySource activitySource, string activityName, Func call, string? traceParent = default, bool exceptionHasPii = true) + { + using ActivityWithPii? activity = StartActivityWithPiiInternal(activityName, activitySource, traceParent); + try + { + await call(activity); + if (activity?.Status == ActivityStatusCode.Unset) activity?.SetStatus(ActivityStatusCode.Ok); + } + catch (Exception ex) + { + TraceExceptionWithPii(ex, activity, exceptionHasPii); + throw; + } + } + + /// + /// Invokes the delegate within the context of a new started . + /// + /// The return type for the delegate. + /// The to start the on. + /// The name of the activity to start. + /// The delegate to call within the context of a newly started + /// The trace parent context, which is used to link the activity to a distributed trace. + /// An indicator that should be set to true if the exception contains PII data and should be treated accordingly when added to the activity. + /// The result of the call to the delegate. + /// + /// Creates and starts a new object if there is any listener for the ActivitySource. + /// Passes the Activity to the delegate and invokes the delegate. If there are no exceptions thrown by the delegate the + /// Activity status is set to and the result is returned. + /// If an exception is thrown by the delegate, the Activity status is set to + /// and an Event is added to the activity and finally the exception is rethrown. + /// + public static async Task TraceActivityAsync(ActivitySource activitySource, string activityName, Func> call, string? traceParent = default, bool exceptionHasPii = true) + { + using ActivityWithPii? activity = StartActivityWithPiiInternal(activityName, activitySource, traceParent); + try + { + T? result = await call(activity); + if (activity?.Status == ActivityStatusCode.Unset) activity?.SetStatus(ActivityStatusCode.Ok); + return result; + } + catch (Exception ex) + { + TraceExceptionWithPii(ex, activity, exceptionHasPii); + throw; + } + } + /// /// Gets or sets the trace parent context. /// @@ -257,6 +456,20 @@ public static async Task TraceActivityAsync(ActivitySource activitySource, private static void TraceException(Exception exception, Activity? activity) => WriteTraceException(exception, activity); + /// + /// Writes the exception to the trace by adding an exception event to the current activity (span). + /// + /// The exception to trace. + /// The current activity where the exception is caught. + /// + /// An indicator that should be set to true if the exception event is recorded + /// at a point where it is known that the exception is escaping the scope of the span/activity. + /// For example, escaped should be true if the exception is caught and re-thrown. + /// However, escaped should be set to false if execution continues in the current scope. + /// + private static void TraceExceptionWithPii(Exception exception, ActivityWithPii? activity, bool exceptionHasPii) => + WriteTraceExceptionWithPii(exception, activity, exceptionHasPii); + /// public void Dispose() { @@ -269,12 +482,24 @@ private static void WriteTraceException(Exception exception, Activity? activity) activity?.SetStatus(ActivityStatusCode.Error); } + private static void WriteTraceExceptionWithPii(Exception exception, ActivityWithPii? activity, bool exceptionHasPii) + { + activity?.AddException(exception, exceptionHasPii: exceptionHasPii); + activity?.SetStatus(ActivityStatusCode.Error); + } + private static Activity? StartActivityInternal(string? activityName, ActivitySource activitySource, string? traceParent = default) { string fullActivityName = GetActivityName(activityName); return StartActivity(activitySource, fullActivityName, traceParent); } + private static ActivityWithPii? StartActivityWithPiiInternal(string? activityName, ActivitySource activitySource, string? traceParent = default) + { + string fullActivityName = GetActivityName(activityName); + return StartActivityWithPii(activitySource, fullActivityName, traceParent); + } + private static string GetActivityName(string? activityName) { if (string.IsNullOrWhiteSpace(activityName)) @@ -296,5 +521,19 @@ private static string GetActivityName(string? activityName) ? (activitySource.StartActivity(activityName, ActivityKind.Client, parentContext)) : (activitySource.StartActivity(activityName, ActivityKind.Client)); } + + /// + /// Creates and starts a new object if there is any listener to the Activity, returns null otherwise. + /// + /// The from which to start the activity. + /// The name of the method for the activity + /// Returns a new object if there is any listener to the Activity, returns null otherwise + private static ActivityWithPii? StartActivityWithPii(ActivitySource activitySource, string activityName, string? traceParent = default) + { + Activity? activity = traceParent != null && ActivityContext.TryParse(traceParent, null, isRemote: true, out ActivityContext parentContext) + ? (activitySource.StartActivity(activityName, ActivityKind.Client, parentContext)) + : (activitySource.StartActivity(activityName, ActivityKind.Client)); + return ActivityWithPii.New(activity); + } } } diff --git a/csharp/src/Apache.Arrow.Adbc/Tracing/ActivityWithPii.cs b/csharp/src/Apache.Arrow.Adbc/Tracing/ActivityWithPii.cs new file mode 100644 index 0000000000..ae8530de03 --- /dev/null +++ b/csharp/src/Apache.Arrow.Adbc/Tracing/ActivityWithPii.cs @@ -0,0 +1,331 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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. + */ + +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.Reflection; +using System.Text; + +namespace Apache.Arrow.Adbc.Tracing +{ + public class ActivityWithPii : IDisposable + { + private readonly Activity _activity; + private readonly bool _shouldDisposeActivity; + private bool _disposed; + + private ActivityWithPii(Activity activity, bool shouldDisposeActivity) + { + _activity = activity; + _shouldDisposeActivity = shouldDisposeActivity; + } + + /// + /// Creates a new instance of from the given .The new instance + /// will call Dispose on the provided activity when this instance is disposed. + /// + /// Use the method in the context of , where you don't own the object and don't want + /// to dispose it. Use the method when starting a new activity via , which returns an that + /// should be disposed when the activity is stopped. + /// + /// + /// The to encapsulate. + /// A new instance of + public static ActivityWithPii? New(Activity? activity) => activity == null ? null : new ActivityWithPii(activity, shouldDisposeActivity: true); + + /// + /// Creates a wrapper instance of for a given . It will not dispose the + /// provided activity. + /// + /// Use the method in the context of , where you don't own the object and don't want + /// to dispose it. Use the method when starting a new activity via , which returns an that + /// should be disposed when the activity is stopped. + /// + /// + /// The to wrap. + /// A new instance of + public static ActivityWithPii? Wrap(Activity? activity) => activity == null ? null : new ActivityWithPii(activity, shouldDisposeActivity: false); + + /// + /// This is an ID that is specific to a particular request. Filtering + /// to a particular ID insures that you get only one request that matches. + /// Id has a hierarchical structure: '|root-id.id1_id2.id3_' Id is generated when + /// is called by appending suffix to Parent.Id + /// or ParentId; Activity has no Id until it started + /// + /// See for more details + /// + /// + /// Id looks like '|a000b421-5d183ab6.1.8e2d4c28_1.': + /// - '|a000b421-5d183ab6.' - Id of the first, top-most, Activity created + /// - '|a000b421-5d183ab6.1.' - Id of a child activity. It was started in the same process as the first activity and ends with '.' + /// - '|a000b421-5d183ab6.1.8e2d4c28_' - Id of the grand child activity. It was started in another process and ends with '_' + /// 'a000b421-5d183ab6' is a for the first Activity and all its children + /// + public string? Id => _activity.Id; + + /// + /// Gets status code of the current activity object. + /// + public ActivityStatusCode Status => _activity.Status; + + /// + /// Holds the W3C 'tracestate' header as a string. + /// + /// Tracestate is intended to carry information supplemental to trace identity contained + /// in traceparent. List of key value pairs carried by tracestate convey information + /// about request position in multiple distributed tracing graphs. It is typically used + /// by distributed tracing systems and should not be used as a general purpose baggage + /// as this use may break correlation of a distributed trace. + /// + /// Logically it is just a kind of baggage (if flows just like baggage), but because + /// it is expected to be special cased (it has its own HTTP header), it is more + /// convenient/efficient if it is not lumped in with other baggage. + /// + public string? TraceStateString + { + get => _activity.TraceStateString; + set => _activity.TraceStateString = value; + } + + /// + /// Update the Activity to have baggage with an additional 'key' and value 'value'. + /// This shows up in the enumeration as well as the + /// method. + /// Baggage is meant for information that is needed for runtime control. For information + /// that is simply useful to show up in the log with the activity use . + /// Returns 'this' for convenient chaining. + /// + /// for convenient chaining. + public ActivityWithPii AddBaggage(string key, string? value, bool isPii = true) + { + bool shouldRedact = isPii; + _activity.AddBaggage(key, shouldRedact ? RedactedValue.DefaultValue : value); + return this; + } + + public ActivityWithPii AddEvent(ActivityEvent e, bool isPii = true) + { + ActivityEvent clone = new(e.Name, e.Timestamp, [.. isPii ? CloneTagsWithRedaction(e.Tags) : e.Tags]); + _activity.AddEvent(clone); + return this; + } + + /// + /// Add a new object to the list. + /// + /// The name of the event. + /// The optional list of tags to attach to the event. + /// for convenient chaining. + public ActivityWithPii AddEvent(string eventName, IReadOnlyList>? tags = default, bool isPii = true) + { + ActivityTagsCollection? tagsCollection = tags == null ? null : [.. tags]; + return AddEvent(new ActivityEvent(eventName, tags: tagsCollection), isPii); + } + + public ActivityWithPii AddException(Exception exception, in TagList tags = default, DateTimeOffset timestamp = default, bool exceptionHasPii = true) + { + if (exception == null) + { + throw new ArgumentNullException(nameof(exception)); + } + + if (!exceptionHasPii) + { + _activity.AddException(exception, tags, timestamp); + return this; + } + + TagList exceptionTags = tags; + const string ExceptionMessageTag = "exception.message"; + bool hasMessage = false; + + for (int i = 0; i < exceptionTags.Count; i++) + { + if (exceptionTags[i].Key == ExceptionMessageTag) + { + exceptionTags[i] = new KeyValuePair( + exceptionTags[i].Key, + exceptionTags[i].Value is RedactedValue ? exceptionTags[i].Value : new RedactedValue(exceptionTags[i].Value)); + hasMessage = true; + } + + // TODO: Decide how to handle other unknown tags that may contain PII. + // For now, we only handle the well-known "exception.message" tag, but there may be other tags that also contain PII and should be redacted. + } + + if (!hasMessage) + { + exceptionTags.Add(new KeyValuePair(ExceptionMessageTag, new RedactedValue(exception.Message))); + } + + _activity.AddException(exception, exceptionTags, timestamp); + return this; + } + + /// + /// Add an to the list. + /// + /// The to add. + /// for convenient chaining. + /// + /// For contexts that are available during span creation, adding links at span creation is preferred to calling later, + /// because head sampling decisions can only consider information present during span creation. + /// + public ActivityWithPii AddLink(ActivityLink link, bool isPii = true) + { + if (link.Tags == null) + { + _activity.AddLink(link); + return this; + } + + ActivityLink clone = new(link.Context, [.. isPii ? CloneTagsWithRedaction(link.Tags) : link.Tags]); + _activity.AddLink(clone); + return this; + } + + /// + /// Add an to the list. + /// + /// + /// + /// + /// + public ActivityWithPii AddLink(string traceParent, IReadOnlyList>? tags = default, bool isPii = true) + { + ActivityTagsCollection? tagsCollection = tags == null ? null : [.. tags]; + return AddLink(new ActivityLink(ActivityContext.Parse(traceParent, null), tags: tagsCollection), isPii); + } + + /// + /// Update the Activity to have a tag with an additional 'key' and value 'value'. + /// This shows up in the enumeration. It is meant for information that + /// is useful to log but not needed for runtime control (for the latter, ) + /// + /// Note: Treating string tags the same as any other object. + /// + /// + /// for convenient chaining. + /// The tag key name + /// The tag value mapped to the input key + public ActivityWithPii AddTag(string key, object? value, bool isPii = true) + { + object? finalValue = value; + switch (value) + { + case Delegate: + MethodInfo methodInfo = value.GetType().GetMethod("Invoke")!; + if (methodInfo.GetParameters().Length != 0) + { + throw new NotSupportedException("Only parameterless delegates are supported as tag values"); + } + object? returnValue = methodInfo.Invoke(value, []); + return AddTag(key, returnValue, isPii); + } + + bool shouldRedact = (isPii && finalValue is not RedactedValue); + _activity.AddTag(key, shouldRedact ? new RedactedValue(finalValue) : finalValue); + return this; + } + + /// + /// Add or update the Activity tag with the input key and value. + /// If the input value is null + /// - if the collection has any tag with the same key, then this tag will get removed from the collection. + /// - otherwise, nothing will happen and the collection will not change. + /// If the input value is not null + /// - if the collection has any tag with the same key, then the value mapped to this key will get updated with the new input value. + /// - otherwise, the key and value will get added as a new tag to the collection. + /// + /// The tag key name + /// The tag value mapped to the input key + /// for convenient chaining. + public ActivityWithPii SetTag(string key, object? value, bool isPii = true) + { + bool shouldRedact = (isPii && value is not RedactedValue); + _activity.SetTag(key, shouldRedact ? new RedactedValue(value) : value); + return this; + } + + /// + /// Sets the status code and description on the current activity object. + /// + /// The status code + /// The error status description + /// for convenient chaining. + /// + /// When passing code value different than ActivityStatusCode.Error, the Activity.StatusDescription will reset to null value. + /// The description parameter will be respected only when passing ActivityStatusCode.Error value. + /// + public void SetStatus(ActivityStatusCode statusCode, string? description = null) + { + _activity.SetStatus(statusCode, description); + } + + protected virtual void Dispose(bool disposing) + { + if (!_disposed) + { + if (disposing) + { + if (_shouldDisposeActivity) + { + _activity.Dispose(); + } + } + + _disposed = true; + } + } + + /// + public void Dispose() + { + // Do not change this code. Put cleanup code in 'Dispose(bool disposing)' method + Dispose(disposing: true); + GC.SuppressFinalize(this); + } + + private static IEnumerable> CloneTagsWithRedaction(IEnumerable> tags) + { + foreach (KeyValuePair tag in tags) + { + yield return new KeyValuePair(tag.Key, tag.Value is RedactedValue ? tag.Value : new RedactedValue(tag.Value)); + } + } + +#if NET5_0_OR_GREATER + public static string ToHexString(byte[] value) => Convert.ToHexString(value); +#else + public static string ToHexString(byte[] value) + { + if (value.Length == 16) + { + return new Guid(value).ToString("N"); + } + StringBuilder hex = new(value.Length * 2); + foreach (byte b in value) + { + hex.AppendFormat("{0:x2}", b); + } + return hex.ToString(); + } +#endif + } +} diff --git a/csharp/src/Apache.Arrow.Adbc/Tracing/IActivityTracerExtensions.cs b/csharp/src/Apache.Arrow.Adbc/Tracing/IActivityTracerExtensions.cs index a71b709e1d..26376ddfca 100644 --- a/csharp/src/Apache.Arrow.Adbc/Tracing/IActivityTracerExtensions.cs +++ b/csharp/src/Apache.Arrow.Adbc/Tracing/IActivityTracerExtensions.cs @@ -24,6 +24,8 @@ namespace Apache.Arrow.Adbc.Tracing { public static class IActivityTracerExtensions { + #region Obsolete methods + /// /// Invokes the delegate within the context of a new started . /// @@ -37,6 +39,7 @@ public static class IActivityTracerExtensions /// status is set to and an Activity is added to the activity /// and finally the exception is rethrown. /// + // TODO: [Obsolete("This method is deprecated. Please use TraceActivity overloads that take ActivityWithPii instead of Activity to avoid accidentally logging PII data.")] public static void TraceActivity(this IActivityTracer tracer, Action call, [CallerMemberName] string? activityName = default, string? traceParent = default) { tracer.Trace.TraceActivity(call, activityName, traceParent ?? tracer.TraceParent); @@ -56,6 +59,7 @@ public static void TraceActivity(this IActivityTracer tracer, Action /// If an exception is thrown by the delegate, the Activity status is set to /// and an Event is added to the activity and finally the exception is rethrown. /// + // TODO: [Obsolete("This method is deprecated. Please use TraceActivity overloads that take ActivityWithPii instead of Activity to avoid accidentally logging PII data.")] public static T TraceActivity(this IActivityTracer tracer, Func call, [CallerMemberName] string? activityName = null, string? traceParent = null) { Type type = typeof(T); @@ -80,6 +84,7 @@ public static T TraceActivity(this IActivityTracer tracer, Func /// If an exception is thrown by the delegate, the Activity status is set to /// and an Event is added to the activity and finally the exception is rethrown. /// + // TODO: [Obsolete("This method is deprecated. Please use TraceActivity overloads that take ActivityWithPii instead of Activity to avoid accidentally logging PII data.")] public static Task TraceActivityAsync(this IActivityTracer tracer, Func call, [CallerMemberName] string? activityName = null, string? traceParent = null) { return tracer.Trace.TraceActivityAsync(call, activityName, traceParent ?? tracer.TraceParent); @@ -99,9 +104,120 @@ public static Task TraceActivityAsync(this IActivityTracer tracer, Func /// and an Event is added to the activity and finally the exception is rethrown. /// + // TODO: [Obsolete("This method is deprecated. Please use TraceActivity overloads that take ActivityWithPii instead of Activity to avoid accidentally logging PII data.")] public static Task TraceActivityAsync(this IActivityTracer tracer, Func> call, [CallerMemberName] string? activityName = null, string? traceParent = null) { return tracer.Trace.TraceActivityAsync(call, activityName, traceParent ?? tracer.TraceParent); } + + #endregion + + /// + /// Invokes the delegate within the context of a new started . + /// + /// The name of the activity. + /// The delegate to call within the context of a newly started + /// The trace parent context, which is used to link the activity to a distributed trace. + /// An indicator that should be set to true if the exception contains PII data and should be treated accordingly when added to the activity. + /// Returns a new object if there is any listener to the Activity, returns null otherwise + /// + /// Creates and starts a new object if there is any listener for the ActivitySource. + /// Passes the Activity to the delegate and invokes the delegate. If there are no exceptions thrown by the delegate the + /// Activity status is set to . If an exception is thrown by the delegate, the Activity + /// status is set to and an Activity is added to the activity + /// and finally the exception is rethrown. + /// + public static void TraceActivity( + this IActivityTracer tracer, + string activityName, + Action call, + string? traceParent = default, + bool exceptionHasPii = true) + { + tracer.Trace.TraceActivity(activityName, call, traceParent ?? tracer.TraceParent, exceptionHasPii); + } + + /// + /// Invokes the delegate within the context of a new started . + /// + /// The return type for the delegate. + /// The name of the activity. + /// The delegate to call within the context of a newly started + /// The trace parent context, which is used to link the activity to a distributed trace. + /// An indicator that should be set to true if the exception contains PII data and should be treated accordingly when added to the activity. + /// The result of the call to the delegate. + /// + /// Creates and starts a new object if there is any listener for the ActivitySource. + /// Passes the Activity to the delegate and invokes the delegate. If there are no exceptions thrown by the delegate the + /// Activity status is set to and the result is returned. + /// If an exception is thrown by the delegate, the Activity status is set to + /// and an Event is added to the activity and finally the exception is rethrown. + /// + public static T TraceActivity( + this IActivityTracer tracer, + string activityName, + Func call, + string? traceParent = null, + bool exceptionHasPii = true) + { + Type type = typeof(T); + if (type == typeof(Task) || (type.IsGenericType && type.GetGenericTypeDefinition() == typeof(Task<>))) + { + throw new InvalidOperationException($"Invalid return type ('{type.Name}') for synchronous method call. Please use {nameof(TraceActivityAsync)}"); + } + + return tracer.Trace.TraceActivity(activityName, call, traceParent ?? tracer.TraceParent, exceptionHasPii); + } + + /// + /// Invokes the delegate within the context of a new started . + /// + /// The name of the activity. + /// The delegate to call within the context of a newly started + /// The trace parent context, which is used to link the activity to a distributed trace. + /// An indicator that should be set to true if the exception contains PII data and should be treated accordingly when added to the activity. + /// + /// + /// Creates and starts a new object if there is any listener for the ActivitySource. + /// Passes the Activity to the delegate and invokes the delegate. If there are no exceptions thrown by the delegate the + /// Activity status is set to and the result is returned. + /// If an exception is thrown by the delegate, the Activity status is set to + /// and an Event is added to the activity and finally the exception is rethrown. + /// + public static Task TraceActivityAsync( + this IActivityTracer tracer, + string activityName, + Func call, + string? traceParent = null, + bool exceptionHasPii = true) + { + return tracer.Trace.TraceActivityAsync(activityName, call, traceParent ?? tracer.TraceParent, exceptionHasPii); + } + + /// + /// Invokes the delegate within the context of a new started . + /// + /// The return type for the delegate. + /// The name of the activity. + /// The delegate to call within the context of a newly started + /// The trace parent context, which is used to link the activity to a distributed trace. + /// An indicator that should be set to true if the exception contains PII data and should be treated accordingly when added to the activity. + /// The result of the call to the delegate. + /// + /// Creates and starts a new object if there is any listener for the ActivitySource. + /// Passes the Activity to the delegate and invokes the delegate. If there are no exceptions thrown by the delegate the + /// Activity status is set to and the result is returned. + /// If an exception is thrown by the delegate, the Activity status is set to + /// and an Event is added to the activity and finally the exception is rethrown. + /// + public static Task TraceActivityAsync( + this IActivityTracer tracer, + string activityName, + Func> call, + string? traceParent = null, + bool exceptionHasPii = true) + { + return tracer.Trace.TraceActivityAsync(activityName, call, traceParent ?? tracer.TraceParent, exceptionHasPii); + } } } diff --git a/csharp/src/Apache.Arrow.Adbc/Tracing/RedactedValue.cs b/csharp/src/Apache.Arrow.Adbc/Tracing/RedactedValue.cs new file mode 100644 index 0000000000..e32edf2fec --- /dev/null +++ b/csharp/src/Apache.Arrow.Adbc/Tracing/RedactedValue.cs @@ -0,0 +1,51 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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. + */ + +using System.Text.Json.Serialization; + +namespace Apache.Arrow.Adbc.Tracing +{ + /// + /// Stores a value that should be redacted when converted to string or serialized to JSON. + /// The value can still be retrieved using the GetValue method. + /// + /// + [JsonConverter(typeof(ToStringJsonConverter))] + public class RedactedValue(object? value) + { + private readonly object? _value = value; + + public const string DefaultValue = "[REDACTED]"; + + /// + /// Returns a string representation of the redacted value. This will always return the default value + /// regardless of the actual value stored in the object. + /// + /// + public override string ToString() => DefaultValue; + + /// + /// Gets the actual value stored in the object. This method can be used to retrieve the original value if needed, + /// but it should be used with caution as it may contain sensitive information. + /// + /// + public object? GetValue() + { + return _value; + } + } +} diff --git a/csharp/src/Apache.Arrow.Adbc/Tracing/ToStringJsonConverter.cs b/csharp/src/Apache.Arrow.Adbc/Tracing/ToStringJsonConverter.cs new file mode 100644 index 0000000000..abd76b513b --- /dev/null +++ b/csharp/src/Apache.Arrow.Adbc/Tracing/ToStringJsonConverter.cs @@ -0,0 +1,40 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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. + */ + +using System; +using System.Text.Json; +using System.Text.Json.Serialization; + +namespace Apache.Arrow.Adbc.Tracing +{ + /// + /// Converts an object to a JSON string using its ToString() method. + /// This is useful for types that do not have a default JSON converter + /// or when you want to control how the object is represented in JSON. + /// + /// + public class ToStringJsonConverter : JsonConverter + { + public override T? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) => + throw new NotImplementedException(); + + public override void Write(Utf8JsonWriter writer, T value, JsonSerializerOptions _) + { + writer.WriteStringValue(value?.ToString()); + } + } +} diff --git a/csharp/src/Telemetry/Traces/Exporters/FileExporter/FileExporter.cs b/csharp/src/Telemetry/Traces/Exporters/FileExporter/FileExporter.cs index 08d4336ab1..2c04f99bb2 100644 --- a/csharp/src/Telemetry/Traces/Exporters/FileExporter/FileExporter.cs +++ b/csharp/src/Telemetry/Traces/Exporters/FileExporter/FileExporter.cs @@ -38,6 +38,13 @@ internal class FileExporter : BaseExporter private static readonly ConcurrentDictionary> s_fileExporters = new(); private static readonly byte[] s_newLine = Encoding.UTF8.GetBytes(Environment.NewLine); + private static readonly JsonSerializerOptions s_serializerOptions = new() + { + Converters = { + // Unredacts any redacted values in the trace when serializing to JSON, so that the full value is available in the file. This is needed since the file exporter is opt-in and users would expect to see the full value in the file. + new UnredactConverter(), + }, + }; private readonly TracingFile _tracingFile; private readonly string _fileBaseName; diff --git a/csharp/src/Telemetry/Traces/Listeners/Apache.Arrow.Adbc.Telemetry.Traces.Listeners.csproj b/csharp/src/Telemetry/Traces/Listeners/Apache.Arrow.Adbc.Telemetry.Traces.Listeners.csproj index d45d327253..e104827170 100644 --- a/csharp/src/Telemetry/Traces/Listeners/Apache.Arrow.Adbc.Telemetry.Traces.Listeners.csproj +++ b/csharp/src/Telemetry/Traces/Listeners/Apache.Arrow.Adbc.Telemetry.Traces.Listeners.csproj @@ -15,4 +15,8 @@ + + + + diff --git a/csharp/src/Telemetry/Traces/Listeners/FileListener/ActivityProcessor.cs b/csharp/src/Telemetry/Traces/Listeners/FileListener/ActivityProcessor.cs index 20fff58408..16ee74c9e0 100644 --- a/csharp/src/Telemetry/Traces/Listeners/FileListener/ActivityProcessor.cs +++ b/csharp/src/Telemetry/Traces/Listeners/FileListener/ActivityProcessor.cs @@ -29,6 +29,13 @@ namespace Apache.Arrow.Adbc.Telemetry.Traces.Listeners.FileListener internal sealed class ActivityProcessor : IDisposable { private static readonly byte[] s_newLine = Encoding.UTF8.GetBytes(Environment.NewLine); + private static readonly JsonSerializerOptions s_serializerOptions = new() + { + Converters = { + // Unredacts any redacted values in the trace when serializing to JSON, so that the full value is available in the file. This is needed since the file exporter is opt-in and users would expect to see the full value in the file. + new UnredactConverter(), + } + }; private Task? _processingTask; private readonly Channel _channel; private readonly Func _streamWriterFunc; diff --git a/csharp/src/Telemetry/Traces/Listeners/FileListener/SerializableActivity.cs b/csharp/src/Telemetry/Traces/Listeners/FileListener/SerializableActivity.cs index 2af071a681..9134bb6e24 100644 --- a/csharp/src/Telemetry/Traces/Listeners/FileListener/SerializableActivity.cs +++ b/csharp/src/Telemetry/Traces/Listeners/FileListener/SerializableActivity.cs @@ -98,8 +98,8 @@ internal SerializableActivity(Activity activity) : this( activity.ParentSpanId, activity.IdFormat, activity.TagObjects.ToDictionary(kv => kv.Key, kv => kv.Value), - activity.Events.Select(e => (SerializableActivityEvent)e).ToArray(), - activity.Links.Select(l => (SerializableActivityLink)l).ToArray(), + [.. activity.Events.Select(e => (SerializableActivityEvent)e)], + [.. activity.Links.Select(l => (SerializableActivityLink)l)], activity.Baggage.ToDictionary(kv => kv.Key, kv => kv.Value)) { } @@ -148,7 +148,7 @@ public static implicit operator SerializableActivityEvent(ActivityEvent source) { Name = source.Name, Timestamp = source.Timestamp, - Tags = source.Tags.ToArray(), + Tags = [.. source.Tags], }; } } diff --git a/csharp/src/Telemetry/Traces/Listeners/FileListener/UnredactConverter.cs b/csharp/src/Telemetry/Traces/Listeners/FileListener/UnredactConverter.cs new file mode 100644 index 0000000000..e5f771d726 --- /dev/null +++ b/csharp/src/Telemetry/Traces/Listeners/FileListener/UnredactConverter.cs @@ -0,0 +1,34 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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. + */ + +using System; +using System.Text.Json; +using System.Text.Json.Serialization; +using Apache.Arrow.Adbc.Tracing; + +namespace Apache.Arrow.Adbc.Telemetry.Traces.Listeners.FileListener +{ + public class UnredactConverter : JsonConverter + { + public override bool CanConvert(Type typeToConvert) => typeToConvert == typeof(RedactedValue); + public override RedactedValue? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) => throw new NotImplementedException(); + public override void Write(Utf8JsonWriter writer, RedactedValue value, JsonSerializerOptions options) + { + writer.WriteRawValue(JsonSerializer.Serialize(value.GetValue(), options)); + } + } +} diff --git a/csharp/test/Apache.Arrow.Adbc.Tests/Tracing/ActivityWithPiiTests.cs b/csharp/test/Apache.Arrow.Adbc.Tests/Tracing/ActivityWithPiiTests.cs new file mode 100644 index 0000000000..89a65b7126 --- /dev/null +++ b/csharp/test/Apache.Arrow.Adbc.Tests/Tracing/ActivityWithPiiTests.cs @@ -0,0 +1,170 @@ +/* +* Licensed to the Apache Software Foundation (ASF) under one or more +* contributor license agreements. See the NOTICE file distributed with +* this work for additional information regarding copyright ownership. +* The ASF licenses this file to You 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. +*/ + +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.Linq; +using Apache.Arrow.Adbc.Tracing; +using Xunit; + +namespace Apache.Arrow.Adbc.Testing.Tracing +{ + + public class ActivityWithPiiTests + { + private class RedactedValueTestData : TheoryData + { + public RedactedValueTestData() + { + Add(null, null); + Add((sbyte)1, (sbyte)1); + Add((byte)1, (byte)1); + Add((short)1, (short)1); + Add((ushort)1, (ushort)1); + Add((int)1, (int)1); + Add((uint)1, (uint)1); + Add((long)1, (long)1); + Add((ulong)1, (ulong)1); + Add((decimal)1, (decimal)1); + Add((float)1, (float)1); + Add((double)1, (double)1); + Add(true, true); + Add('A', 'A'); + Add("string", "string" ); + Add(new object?[] { null }, new object?[] { null }); + Add(new object?[] { (sbyte)1 }, new object?[] { (sbyte)1 }); + Add(new object?[] { (byte)1 }, new object?[] { (byte)1 }); + Add(new object?[] { (short)1 }, new object?[] { (short)1 }); + Add(new object?[] { (ushort)1 }, new object?[] { (ushort)1 }); + Add(new object?[] { (int)1 }, new object?[] { (int)1 }); + Add(new object?[] { (uint)1 }, new object?[] { (uint)1 }); + Add(new object?[] { (long)1 }, new object?[] { (long)1 }); + Add(new object?[] { (ulong)1 }, new object?[] { (ulong)1 }); + Add(new object?[] { (decimal)1 }, new object?[] { (decimal)1 }); + Add(new object?[] { (float)1 }, new object?[] { (float)1 }); + Add(new object?[] { (double)1 }, new object?[] { (double)1 }); + Add(new object?[] { true }, new object?[] { true }); + Add(new object?[] { 'A' }, new object?[] { 'A' }); + Add(new object?[] { "string" }, new object?[] { "string" }); + } + } + + [SkippableTheory] + [ClassData(typeof(RedactedValueTestData))] + public void CanAddTagDefaultIsPii(object? testValue, object? expectedValue) + { + string activityName = NewName(); + Activity activity = new(activityName); + var activityWithPii = ActivityWithPii.New(activity); + + Assert.NotNull(activityWithPii); + activityWithPii.AddTag("keyName", testValue); + var value = activity.TagObjects.First().Value as RedactedValue; + Assert.NotNull(value); + Assert.Equal(expectedValue, value.GetValue()); + Assert.Equal(RedactedValue.DefaultValue, value.ToString()); + } + + [SkippableFact] + public void CanAddTagUsingDelegateDefaultIsPii() + { + List<(object? TestValue, object? ExpectedValue)> testData = []; + testData.Add((() => new object?[] { "string" }, new object?[] { "string" })); + testData.Add((() => new object?[] { 1 }, new object?[] { 1 })); + testData.Add((() => new object?[] { null }, new object?[] { null })); + testData.Add((getValue(1), "1")); + testData.Add((getAnotherValue(getValue(1)), "1")); + + static Func getAnotherValue(Func anotherFunction) => () => anotherFunction(); + static Func getValue(int localValue) => localValue.ToString; + + string activityName = NewName(); + Activity activity = new(activityName); + var activityWithPii = ActivityWithPii.New(activity); + int index = 0; + Assert.NotNull(activityWithPii); + + foreach ((object? testValue, object? expectedValue) in testData) + { + string key = "keyName" + index++; + activityWithPii = activityWithPii.AddTag(key, testValue); + Assert.Contains(activity.TagObjects, e => e.Key == key); + var value = activity.TagObjects.Where(e => e.Key == key).First().Value as RedactedValue; + Assert.NotNull(value); + Assert.Equal(expectedValue, value.GetValue()); + Assert.Equal(RedactedValue.DefaultValue, value.ToString()); + } + } + + [SkippableTheory] + [ClassData(typeof(RedactedValueTestData))] + public void CanAddTagIsPiiAsFalse(object? testValue, object? expectedValue) + { + string activityName = NewName(); + Activity activity = new(activityName); + var activityWithPii = ActivityWithPii.New(activity); + Assert.NotNull(activityWithPii); + + activityWithPii.AddTag("keyName", testValue, isPii: false); + object? value = activity.TagObjects.First().Value; + if (expectedValue == null) + { + Assert.Null(value); + } + else + { + Assert.NotNull(value); + Assert.IsNotType(value); + } + Assert.Equal(expectedValue, value); + } + + [SkippableFact] + public void CanAddTagUsingDelegateIsPiiFalse() + { + List<(object? TestValue, object? ExpectedValue)> testData = []; + testData.Add((() => new object?[] { "string" }, new object?[] { "string" })); + testData.Add((() => new object?[] { 1 }, new object?[] { 1 })); + testData.Add((() => new object?[] { null }, new object?[] { null })); + testData.Add((getValue(1), "1")); + testData.Add((getAnotherValue(getValue(1)), "1")); + + static Func getAnotherValue(Func anotherFunction) => () => anotherFunction(); + static Func getValue(int localValue) => localValue.ToString; + + string activityName = NewName(); + Activity activity = new(activityName); + var activityWithPii = ActivityWithPii.New(activity); + int index = 0; + Assert.NotNull(activityWithPii); + + foreach ((object? testValue, object? expectedValue) in testData) + { + string key = "keyName" + index++; + activityWithPii = activityWithPii.AddTag(key, testValue, false); + Assert.Contains(activity.TagObjects, e => e.Key == key); + object? value = activity.TagObjects.Where(e => e.Key == key).First().Value; + Assert.NotNull(value); + Assert.IsNotType(value); + Assert.Equal(expectedValue, value); + } + } + + private static string NewName() => new Guid().ToString("N"); + } +} diff --git a/csharp/test/Apache.Arrow.Adbc.Tests/Tracing/TracingTests.cs b/csharp/test/Apache.Arrow.Adbc.Tests/Tracing/TracingTests.cs index 021eaed534..d716e137fa 100644 --- a/csharp/test/Apache.Arrow.Adbc.Tests/Tracing/TracingTests.cs +++ b/csharp/test/Apache.Arrow.Adbc.Tests/Tracing/TracingTests.cs @@ -28,7 +28,7 @@ using Xunit; using Xunit.Abstractions; -namespace Apache.Arrow.Adbc.Tests.Tracing +namespace Apache.Arrow.Adbc.Testing.Tracing { public class TracingTests(ITestOutputHelper? outputHelper) : IDisposable { @@ -61,7 +61,7 @@ internal void CanStartActivity() Assert.Equal(currLength, exportedActivities.Count); int lineCount = 0; - foreach (var exportedActivity in exportedActivities) + foreach (Activity exportedActivity in exportedActivities) { lineCount++; Assert.NotNull(exportedActivity); @@ -94,7 +94,7 @@ internal void CanAddEvent() Assert.Equal(currLength, exportedActivities.Count); int lineCount = 0; - foreach (var exportedActivity in exportedActivities) + foreach (Activity exportedActivity in exportedActivities) { lineCount++; Assert.NotNull(exportedActivity); @@ -120,7 +120,7 @@ internal void CanAddActivityWithDepth() testClass.MethodWithActivityRecursive(nameof(TraceProducer.MethodWithActivityRecursive), recurseCount); int lineCount = 0; - foreach (var exportedActivity in exportedActivities) + foreach (Activity exportedActivity in exportedActivities) { lineCount++; Assert.NotNull(exportedActivity); @@ -148,7 +148,7 @@ internal void CanAddTraceParent() const string eventNameWithParent = "eventNameWithParent"; const string eventNameWithoutParent = "eventNameWithoutParent"; testClass.MethodWithActivity(eventNameWithoutParent); - Assert.True(exportedActivities.Count() > 0); + Assert.True(exportedActivities.Count > 0); const int withParentCountExpected = 10; for (int i = 0; i < withParentCountExpected; i++) @@ -157,12 +157,12 @@ internal void CanAddTraceParent() } testClass.MethodWithActivity(eventNameWithoutParent); - Assert.True(exportedActivities.Count() > 0); + Assert.True(exportedActivities.Count > 0); int lineCount = 0; int withParentCount = 0; int withoutParentCount = 0; - foreach (var exportedActivity in exportedActivities) + foreach (Activity exportedActivity in exportedActivities) { lineCount++; Assert.NotNull(exportedActivity); @@ -279,7 +279,7 @@ internal void CanSetTraceParentOnStatement() string eventName1 = NewName(); statement.MethodWithActivity(eventName1); Assert.Single(exportedActivities); - var activity1 = exportedActivities.First(); + Activity activity1 = exportedActivities.First(); Assert.Equal(connectionTraceParent, activity1.ParentId); // Test 2: Set statement-specific trace parent @@ -291,7 +291,7 @@ internal void CanSetTraceParentOnStatement() string eventName2 = NewName(); statement.MethodWithActivity(eventName2); Assert.Single(exportedActivities); - var activity2 = exportedActivities.First(); + Activity activity2 = exportedActivities.First(); Assert.Equal(statementTraceParent, activity2.ParentId); // Test 3: Set trace parent to null (should fall back to connection's trace parent) @@ -302,7 +302,7 @@ internal void CanSetTraceParentOnStatement() string eventName3 = NewName(); statement.MethodWithActivity(eventName3); Assert.Single(exportedActivities); - var activity3 = exportedActivities.First(); + Activity activity3 = exportedActivities.First(); Assert.Equal(connectionTraceParent, activity3.ParentId); } @@ -344,29 +344,29 @@ internal void MethodWithNoInstrumentation() internal void MethodWithActivity() { - _trace.TraceActivity(_ => { }); + _trace.TraceActivity("MethodWithActivity", _ => { }, exceptionHasPii: true); } internal void MethodWithActivity(string activityName, string? traceParent = default) { - _trace.TraceActivity(activity => { }, activityName: activityName, traceParent: traceParent); + _trace.TraceActivity(activityName, activity => { }, traceParent: traceParent, exceptionHasPii: true); } internal void MethodWithActivityRecursive(string activityName, int recurseCount) { - _trace.TraceActivity(_ => + _trace.TraceActivity(activityName + recurseCount.ToString(), _ => { recurseCount--; if (recurseCount > 0) { MethodWithActivityRecursive(activityName, recurseCount); } - }, activityName: activityName + recurseCount.ToString()); + }, exceptionHasPii: true); } internal void MethodWithEvent(string eventName) { - _trace.TraceActivity((activity) => activity?.AddEvent(eventName)); + _trace.TraceActivity("MethodWithEvent", (ActivityWithPii? activity) => activity?.AddEvent(eventName)); } internal void MethodWithAllProperties( @@ -375,16 +375,16 @@ internal void MethodWithAllProperties( IReadOnlyList> tags, string traceParent) { - _trace.TraceActivity(activity => + _trace.TraceActivity(activityName, activity => { foreach (KeyValuePair tag in tags) { activity?.AddTag(tag.Key, tag.Value) .AddBaggage(tag.Key, tag.Value?.ToString()); } - activity?.AddEvent(eventName, tags) + activity?.AddEvent(eventName, tags)? .AddLink(traceParent, tags); - }, activityName: activityName, traceParent: traceParent); + }, traceParent: traceParent, exceptionHasPii: true); } protected virtual void Dispose(bool disposing) @@ -416,7 +416,7 @@ private class MyTracingConnection(IReadOnlyDictionary properties public void MethodWithActivity() { - this.TraceActivity(activity => + this.TraceActivity("MethodWithActivity", (ActivityWithPii? activity) => { activity?.AddTag("exampleTag", "exampleValue") .AddBaggage("exampleBaggage", "exampleBaggageValue") @@ -428,7 +428,7 @@ public void MethodWithActivity() public async Task MethodWithInvalidAsyncTraceActivity1() { // This method is intended to demonstrate incorrect usage of TraceActivity with async methods. - return await this.TraceActivity(async activity => + return await this.TraceActivity("MethodWithInvalidAsyncTraceActivity1", async (ActivityWithPii? activity) => { await Task.Delay(1); return true; @@ -438,7 +438,7 @@ public async Task MethodWithInvalidAsyncTraceActivity1() public async Task MethodWithInvalidAsyncTraceActivity2() { // This method is intended to demonstrate incorrect usage of TraceActivity with async methods. - await this.TraceActivity(async activity => + await this.TraceActivity("MethodWithInvalidAsyncTraceActivity2", async (ActivityWithPii? activity) => { await Task.Delay(1); return; @@ -448,7 +448,7 @@ await this.TraceActivity(async activity => public async ValueTask MethodWithInvalidAsyncTraceActivity3() { // This method is intended to demonstrate incorrect usage of TraceActivity with async methods. - return await this.TraceActivity(async activity => + return await this.TraceActivity("MethodWithInvalidAsyncTraceActivity3", async (ActivityWithPii? activity) => { await Task.Delay(1); return true; @@ -458,7 +458,7 @@ public async ValueTask MethodWithInvalidAsyncTraceActivity3() public async ValueTask MethodWithInvalidAsyncTraceActivity4() { // This method is intended to demonstrate incorrect usage of TraceActivity with async methods. - await this.TraceActivity(async activity => + await this.TraceActivity("MethodWithInvalidAsyncTraceActivity4", async (ActivityWithPii? activity) => { await Task.Delay(1); return; @@ -468,7 +468,7 @@ await this.TraceActivity(async activity => public async Task MethodWithInvalidAsyncTraceActivity5() { // This method is intended to demonstrate incorrect usage of TraceActivity with async methods. - return await this.TraceActivity(async activity => + return await this.TraceActivity("MethodWithInvalidAsyncTraceActivity5", async (ActivityWithPii? activity) => { await Task.Delay(1); return await new AwaitableBool(); @@ -509,10 +509,10 @@ private class MyTracingStatement(TracingConnection connection) : TracingStatemen public void MethodWithActivity(string activityName) { - this.TraceActivity(activity => + this.TraceActivity(activityName, (ActivityWithPii? activity) => { activity?.AddTag("testTag", "testValue"); - }, activityName); + }); } public override void SetOption(string key, string? value) diff --git a/csharp/test/Telemetry/Traces/Listeners/FileListener/FileActivityListenerTests.cs b/csharp/test/Telemetry/Traces/Listeners/FileListener/FileActivityListenerTests.cs index f5bf6c1aa5..39de06fa06 100644 --- a/csharp/test/Telemetry/Traces/Listeners/FileListener/FileActivityListenerTests.cs +++ b/csharp/test/Telemetry/Traces/Listeners/FileListener/FileActivityListenerTests.cs @@ -35,9 +35,18 @@ public class FileActivityListenerTests [InlineData(ListenersOptions.Exporters.AdbcFile, true)] public void TestTryActivateFileListener(string? exporterOption, bool expected) { - Assert.Equal(expected, FileActivityListener.TryActivateFileListener("TestSource", exporterOption, out FileActivityListener? listener)); - Assert.True(expected == (listener != null)); - listener?.Dispose(); + string? previousEnvValue = Environment.GetEnvironmentVariable(ListenersOptions.Environment.Exporter); + try + { + Environment.SetEnvironmentVariable(ListenersOptions.Environment.Exporter, null); + Assert.Equal(expected, FileActivityListener.TryActivateFileListener("TestSource", exporterOption, out FileActivityListener? listener)); + Assert.True(expected == (listener != null)); + listener?.Dispose(); + } + finally + { + Environment.SetEnvironmentVariable(ListenersOptions.Environment.Exporter, previousEnvValue); + } } [Fact] @@ -122,7 +131,7 @@ public TestConnection(IReadOnlyDictionary properties) public async Task EmulateWorkAsync(string key, string value) { - await this.TraceActivityAsync(async (activity) => + await this.TraceActivityAsync("EmulateWorkAsync", async (ActivityWithPii? activity) => { activity?.AddTag(key, value); // Simulate some work diff --git a/csharp/test/Telemetry/Traces/Listeners/FileListener/SerializableActivityTests.cs b/csharp/test/Telemetry/Traces/Listeners/FileListener/SerializableActivityTests.cs new file mode 100644 index 0000000000..d3683b8763 --- /dev/null +++ b/csharp/test/Telemetry/Traces/Listeners/FileListener/SerializableActivityTests.cs @@ -0,0 +1,155 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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. + */ + +using System.Diagnostics; +using System.Text; +using System.Text.Json; +using System.Text.Json.Serialization.Metadata; +using Apache.Arrow.Adbc.Telemetry.Traces.Listeners.FileListener; +using Apache.Arrow.Adbc.Tracing; +using Xunit.Abstractions; + +namespace Apache.Arrow.Adbc.Tests.Telemetry.Traces.Listeners.FileListener +{ + public class SerializableActivityTests + { + private readonly ITestOutputHelper _output; + + public class SerializableActivityTestData : TheoryData + { + public SerializableActivityTestData() + { + var activityWithTags = new Activity("TestActivityWithTags"); + int index = 0; + + activityWithTags.AddTag("key" + index++, "value1"); + activityWithTags.AddTag("key" + index++, (sbyte)123); + activityWithTags.AddTag("key" + index++, (byte)123); + activityWithTags.AddTag("key" + index++, (short)123); + activityWithTags.AddTag("key" + index++, (ushort)123); + activityWithTags.AddTag("key" + index++, (int)123); + activityWithTags.AddTag("key" + index++, (uint)123); + activityWithTags.AddTag("key" + index++, (long)123); + activityWithTags.AddTag("key" + index++, (ulong)123); + activityWithTags.AddTag("key" + index++, (float)123); + activityWithTags.AddTag("key" + index++, (double)123); + activityWithTags.AddTag("key" + index++, (decimal)123); + activityWithTags.AddTag("key" + index++, true); + activityWithTags.AddTag("key" + index++, 'A'); + + activityWithTags.AddTag("key" + index++, new string[] { "val1" }); + activityWithTags.AddTag("key" + index++, new byte[] { 123 }); + activityWithTags.AddTag("key" + index++, new sbyte[] { 123 }); + activityWithTags.AddTag("key" + index++, new ushort[] { 123 }); + activityWithTags.AddTag("key" + index++, new short[] { 123 }); + activityWithTags.AddTag("key" + index++, new uint[] { 123 }); + activityWithTags.AddTag("key" + index++, new int[] { 123 }); + activityWithTags.AddTag("key" + index++, new ulong[] { 123 }); + activityWithTags.AddTag("key" + index++, new long[] { 123 }); + activityWithTags.AddTag("key" + index++, new float[] { 123 }); + activityWithTags.AddTag("key" + index++, new double[] { 123 }); + activityWithTags.AddTag("key" + index++, new decimal[] { 123 }); + activityWithTags.AddTag("key" + index++, new bool[] { true }); + activityWithTags.AddTag("key" + index++, new char[] { 'A' }); + + activityWithTags.AddTag("key" + index++, new string[] { "val1" }.AsEnumerable()); + activityWithTags.AddTag("key" + index++, new byte[] { 123 }.AsEnumerable()); + activityWithTags.AddTag("key" + index++, new sbyte[] { 123 }.AsEnumerable()); + activityWithTags.AddTag("key" + index++, new ushort[] { 123 }.AsEnumerable()); + activityWithTags.AddTag("key" + index++, new short[] { 123 }.AsEnumerable()); + activityWithTags.AddTag("key" + index++, new uint[] { 123 }.AsEnumerable()); + activityWithTags.AddTag("key" + index++, new int[] { 123 }.AsEnumerable()); + activityWithTags.AddTag("key" + index++, new ulong[] { 123 }.AsEnumerable()); + activityWithTags.AddTag("key" + index++, new long[] { 123 }.AsEnumerable()); + activityWithTags.AddTag("key" + index++, new float[] { 123 }.AsEnumerable()); + activityWithTags.AddTag("key" + index++, new double[] { 123 }.AsEnumerable()); + activityWithTags.AddTag("key" + index++, new decimal[] { 123 }.AsEnumerable()); + activityWithTags.AddTag("key" + index++, new bool[] { true }.AsEnumerable()); + activityWithTags.AddTag("key" + index++, new char[] { 'A' }.AsEnumerable()); + + activityWithTags.AddTag("key" + index++, new Uri("http://example.com")); + Add(activityWithTags); + } + } + + public SerializableActivityTests(ITestOutputHelper output) + { + _output = output; + } + + [Fact] + public async Task CanRedactValue() + { + string activityName = NewName(); + Activity activity = new Activity(activityName).Start(); + using ActivityWithPii? activityWithPii = ActivityWithPii.New(activity); + Assert.NotNull(activityWithPii); + string testValue = NewName(); + activityWithPii.AddTag("keyName", new RedactedValue(testValue)); + var value = activity.TagObjects.First().Value as RedactedValue; + Assert.NotNull(value); + Assert.Equal("[REDACTED]", value.ToString()); + Assert.Equal(testValue, value.GetValue()); + + SerializableActivity serializableActivity = new(activity); + var stream = new MemoryStream(); + await JsonSerializer.SerializeAsync( + stream, + serializableActivity); + Assert.NotNull(stream); + string actualString = Encoding.UTF8.GetString(stream.ToArray()); + Assert.DoesNotContain(testValue, actualString); + Assert.Contains("[REDACTED]", actualString); + } + + [Fact] + public async Task CanUnredactValue() + { + string activityName = NewName(); + Activity activity = new Activity(activityName).Start(); + using ActivityWithPii? activityWithPii = ActivityWithPii.New(activity); + Assert.NotNull(activityWithPii); + activity.Start(); + string testValue = NewName(); + activityWithPii.AddTag("keyName", new RedactedValue(testValue)); + var value = activity.TagObjects.First().Value as RedactedValue; + Assert.NotNull(value); + Assert.Equal("[REDACTED]", value.ToString()); + Assert.Equal(testValue, value.GetValue()); + + SerializableActivity serializableActivity = new(activity); + var stream = new MemoryStream(); + JsonSerializerOptions serializerOptions = new() + { + Converters = + { + new UnredactConverter() + }, + }; + await JsonSerializer.SerializeAsync( + stream, + serializableActivity, + serializerOptions); + Assert.NotNull(stream); + string actualString = Encoding.UTF8.GetString(stream.ToArray()); + Assert.Contains(testValue, actualString); + Assert.DoesNotContain("[REDACTED]", actualString); + } + + private string NewName() => Guid.NewGuid().ToString("N"); + } +}