You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "birschick-bq (via GitHub)" <gi...@apache.org> on 2025/07/14 23:20:44 UTC
[PR] feat(charp/src/Apache.Arrow.Adbc): ensure tracing redacts PII [arrow-adbc]
birschick-bq opened a new pull request, #3157:
URL: https://github.com/apache/arrow-adbc/pull/3157
Creates a wrapper class to the `Activity` class such that values traced will, by default, be redacted.
To ensure the value is not redacted, the developer needs to change the `isPii` flag to `false` when instrumenting tracing.
Likewise, the `TraceActivity` methods will by default mark any exception message as PII. Developers can use the `exceptionIsPii` set to `false` to change the behavior.
The `RedactedValue` structure includes the `GetUnredactedValue` method for listeners/exporters that can safely handle the unredacted PII.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(csharp/src/Apache.Arrow.Adbc): ensure tracing redacts PII [arrow-adbc]
Posted by "davidhcoe (via GitHub)" <gi...@apache.org>.
davidhcoe commented on code in PR #3157:
URL: https://github.com/apache/arrow-adbc/pull/3157#discussion_r2309061440
##########
csharp/src/Drivers/BigQuery/BigQueryConnection.cs:
##########
@@ -136,30 +136,30 @@ internal BigQueryClient Open(string? projectId = null)
if (projectId.Equals(BigQueryConstants.PublicProjectId, StringComparison.OrdinalIgnoreCase))
{
projectId = BigQueryConstants.DetectProjectId;
- activity?.AddBigQueryTag("change_public_projectId_to_detect_project_id", projectId);
+ activity?.AddBigQueryTag("change_public_projectId_to_detect_project_id", projectId, isPii: false);
}
}
// the billing project can be null if it's not specified
if (this.properties.TryGetValue(BigQueryParameters.BillingProjectId, out billingProjectId))
{
- activity?.AddBigQueryParameterTag((BigQueryParameters.BillingProjectId), billingProjectId);
+ activity?.AddBigQueryParameterTag((BigQueryParameters.BillingProjectId), billingProjectId, isPii: false);
Review Comment:
I could see this potentially being pii, based on how people name their projects.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(csharp/src/Apache.Arrow.Adbc): ensure tracing redacts PII [arrow-adbc]
Posted by "birschick-bq (via GitHub)" <gi...@apache.org>.
birschick-bq commented on PR #3157:
URL: https://github.com/apache/arrow-adbc/pull/3157#issuecomment-3762207535
Abandoning. Will bring something later.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(csharp/src/Apache.Arrow.Adbc): ensure tracing redacts PII [arrow-adbc]
Posted by "birschick-bq (via GitHub)" <gi...@apache.org>.
birschick-bq commented on code in PR #3157:
URL: https://github.com/apache/arrow-adbc/pull/3157#discussion_r2211228796
##########
csharp/src/Apache.Arrow.Adbc/Tracing/ActivityWithPii.cs:
##########
@@ -0,0 +1,277 @@
+/*
+ * 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 System.Text;
+
+namespace Apache.Arrow.Adbc.Tracing
+{
+ public sealed class ActivityWithPii : IDisposable
+ {
+ private readonly Activity _activity;
+
+ internal ActivityWithPii(Activity activity)
+ {
+ _activity = activity;
+ }
+
+ /// <summary>
+ /// Update the Activity to have baggage with an additional 'key' and value 'value'.
+ /// This shows up in the <see cref="Baggage"/> enumeration as well as the <see cref="GetBaggageItem(string)"/>
+ /// 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 <see cref="Tags"/>.
+ /// Returns 'this' for convenient chaining.
+ /// </summary>
+ /// <returns><see langword="this" /> for convenient chaining.</returns>
+ public ActivityWithPii AddBaggage(string key, RedactedValue value)
+ {
+ _activity.AddBaggage(key, value.ToString());
+ return this;
+ }
+
+ /// <summary>
+ /// Update the Activity to have baggage with an additional 'key' and value 'value'. Use parameter 'isPii' to indicate
+ /// whether the value is PII (Personally Identifiable Information).
+ /// This shows up in the <see cref="Baggage"/> enumeration as well as the <see cref="GetBaggageItem(string)"/>
+ /// 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 <see cref="Tags"/>.
+ /// Returns 'this' for convenient chaining.
+ /// </summary>
+ /// <returns><see langword="this" /> for convenient chaining.</returns>
+ public ActivityWithPii AddBaggage(string key, object? value, bool isPii = true)
+ {
+ _activity.AddBaggage(key, ToRedactedValue(value, isPii).ToString());
+ return this;
+ }
+
+ /// <summary>
+ /// Add <see cref="ActivityEvent" /> object to the <see cref="Events" /> list.
+ /// </summary>
+ /// <param name="e"> object of <see cref="ActivityEvent"/> to add to the attached events list.</param>
+ /// <returns><see langword="this" /> for convenient chaining.</returns>
+ public ActivityWithPii AddEvent(string eventName, IReadOnlyList<KeyValuePair<string, RedactedValue?>>? tags)
+ {
+ ActivityTagsCollection? tagsCollection = ToActivityTagsCollection(tags);
+ _activity.AddEvent(new ActivityEvent(eventName, tags: tagsCollection));
+ return this;
+ }
+
+ /// <summary>
+ /// Add <see cref="ActivityEvent" /> object to the <see cref="Events" /> list. Use parameter 'isPii' to indicate
+ /// whether the tags contain PII (Personally Identifiable Information).
+ /// </summary>
+ /// <param name="e"> object of <see cref="ActivityEvent"/> to add to the attached events list.</param>
+ /// <returns><see langword="this" /> for convenient chaining.</returns>
+ public ActivityWithPii AddEvent(string eventName, IReadOnlyList<KeyValuePair<string, object?>>? tags = default, bool isPii = true)
+ {
+ ActivityTagsCollection? tagsCollection = ToActivityTagsCollection(tags, isPii);
+ _activity.AddEvent(new ActivityEvent(eventName, tags: tagsCollection));
+ return this;
+ }
+
+ /// <summary>
+ /// Add an <see cref="ActivityEvent" /> object containing the exception information to the <see cref="Events" /> list.
+ /// </summary>
+ /// <param name="exception">The exception to add to the attached events list.</param>
+ /// <param name="isPii">Indicates whether the exception message contain PII (Personally Identifiable Information).</param>
+ /// <returns><see langword="this" /> for convenient chaining.</returns>
+ /// <remarks>
+ /// <para>- The name of the event will be "exception", and it will include the tags "exception.message", "exception.stacktrace", and "exception.type".</para>
+ /// <para>- Any registered <see cref="ActivityListener"/> with the <see cref="ActivityListener.ExceptionRecorder"/> callback will be notified about this exception addition
+ /// before the <see cref="ActivityEvent" /> object is added to the <see cref="Events" /> list.</para>
+ /// <para>- Any registered <see cref="ActivityListener"/> with the <see cref="ActivityListener.ExceptionRecorder"/> callback that adds "exception.message", "exception.stacktrace", or "exception.type" tags
+ /// will not have these tags overwritten, except by any subsequent <see cref="ActivityListener"/> that explicitly overwrites them.</para>
+ /// </remarks>
+ public ActivityWithPii AddException(Exception exception, bool isPii = true)
+ {
+ const string ExceptionMessageTag = "exception.message";
+ TagList tagList = new();
+ if (isPii)
+ {
+ tagList.Add(new KeyValuePair<string, object?>(ExceptionMessageTag, new RedactedValue(exception.Message)));
+ }
+
+ _activity.AddException(exception, tagList);
+ return this;
+ }
+
+ /// <summary>
+ /// Add a new <see cref="ActivityLink"/> to the <see cref="Activity.Links"/> list.
+ /// </summary>
+ /// <param name="traceParent">The traceParent id for the associated <see cref="ActivityContext"/>.</param>
+ /// <param name="tags">The optional list of tags to attach to the event.</param>
+ /// <param name="isPii">Indicates whether the tags contain PII (Personally Identifiable Information).</param>
+ /// <returns><see langword="this"/> for convenient chaining.</returns>
+ public ActivityWithPii AddLink(string traceParent, IReadOnlyList<KeyValuePair<string, object?>>? tags = default, bool isPii = true)
+ {
+ ActivityTagsCollection? tagsCollection = ToActivityTagsCollection(tags, isPii);
+ _activity?.AddLink(new ActivityLink(ActivityContext.Parse(traceParent, null), tags: tagsCollection));
+ return this;
+ }
+
+ /// <summary>
+ /// Update the Activity to have a tag with an additional 'key' and value 'value'.
+ /// This shows up in the <see cref="TagObjects"/> enumeration. It is meant for information that
+ /// is useful to log but not needed for runtime control (for the latter, <see cref="Baggage"/>)
+ /// </summary>
+ /// <returns><see langword="this" /> for convenient chaining.</returns>
+ /// <param name="key">The tag key name</param>
+ /// <param name="value">The tag value mapped to the input key</param>
+ /// <param name="isPii">Indicates whether the value is PII (Personally Identifiable Information).</param>
+ public ActivityWithPii AddTag(string key, object? value, bool isPii = true)
+ {
+ _activity.AddTag(key, ToRedactedValue(value, isPii));
+ return this;
+ }
+
+ /// <summary>
+ /// Update the Activity to have a tag with an additional 'key' and value 'value'.
+ /// This shows up in the <see cref="TagObjects"/> enumeration. It is meant for information that
+ /// is useful to log but not needed for runtime control (for the latter, <see cref="Baggage"/>)
+ /// </summary>
+ /// <returns><see langword="this" /> for convenient chaining.</returns>
+ /// <param name="key">The tag key name</param>
+ /// <param name="value">The tag value mapped to the input key</param>
+ public ActivityWithPii AddTag(string key, RedactedValue value)
+ {
+ _activity.AddTag(key, value);
+ return this;
+ }
+
+ /// <summary>
+ /// Update the Activity to have a tag with an additional 'key' and value 'value'.
+ /// This shows up in the <see cref="TagObjects"/> enumeration. It is meant for information that
+ /// is useful to log but not needed for runtime control (for the latter, <see cref="Baggage"/>)
+ /// </summary>
+ /// <returns><see langword="this" /> for convenient chaining.</returns>
+ /// <param name="key">The tag key name</param>
+ /// <param name="valueFn">The tag value function mapped to the input key</param>
+ /// <param name="isPii">Indicates whether the value is PII (Personally Identifiable Information).</param>
+ public ActivityWithPii AddTag(string key, Func<object?> valueFn, bool isPii = true)
+ {
+ _activity.AddTag(key, ToRedactedValue(valueFn(), isPii));
+ return this;
+ }
+
+ /// <summary>
+ /// Update the Activity to have a tag with an additional 'key' and value 'value'.
+ /// This shows up in the <see cref="TagObjects"/> enumeration. It is meant for information that
+ /// is useful to log but not needed for runtime control (for the latter, <see cref="Baggage"/>)
+ /// </summary>
+ /// <returns><see langword="this" /> for convenient chaining.</returns>
+ /// <param name="key">The tag key name</param>
+ /// <param name="value">The tag value mapped to the input key as a function</param>
+ /// <param name="guidFormat">The format indicator for 16-byte GUID arrays.</param>
+ public ActivityWithPii AddTag(string key, byte[]? value, string? guidFormat, bool isPii = true)
+ {
+ if (value == null)
+ {
+ return AddTag(key, value, isPii);
+ }
+ if (value.Length == 16)
+ {
+ return AddTag(key, new Guid(value).ToString(guidFormat), isPii);
+ }
+ return AddTag(key, ToHexString(value), isPii);
+ }
+
+ /// <summary>
+ /// Update the Activity to have a tag with an additional 'key' and value 'value'.
+ /// This shows up in the <see cref="TagObjects"/> enumeration. It is meant for information that
+ /// is useful to log but not needed for runtime control (for the latter, <see cref="Baggage"/>)
+ /// </summary>
+ /// <returns><see langword="this" /> for convenient chaining.</returns>
+ /// <param name="key">The tag key name as a function</param>
+ /// <param name="value">The tag value mapped to the input key</param>
+ /// /// <param name="condition">The condition to check before adding the tag</param>
+ public ActivityWithPii AddConditionalTag(string key, string? value, bool condition, bool isPii = true)
+ {
+ if (condition)
+ {
+ return AddTag(key, value, isPii);
+ }
+
+ return this;
+ }
+
+ /// <summary>
+ /// Sets the status code and description on the current activity object.
+ /// </summary>
+ /// <param name="code">The status code</param>
+ /// <param name="description">The error status description</param>
+ /// <returns><see langword="this" /> for convenient chaining.</returns>
+ /// <remarks>
+ /// 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.
+ /// </remarks>
+ public ActivityWithPii SetStatus(ActivityStatusCode code, string? description = null)
+ {
+ _activity.SetStatus(code, description);
+ return this;
+ }
+
+ /// <summary>
+ /// Gets status code of the current activity object.
+ /// </summary>
+ public ActivityStatusCode Status => _activity.Status;
+
+ /// <summary>
+ /// Dipsoses the current <see cref="Activity"/> object.
+ /// </summary>
+ public void Dispose()
+ {
+ _activity.Dispose();
+ }
+
+ private static RedactedValue ToRedactedValue(object? value, bool isPii) =>
+ value is RedactedValue redactedValue ? redactedValue : new RedactedValue(value);
Review Comment:
Good find. Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(csharp/src/Apache.Arrow.Adbc): ensure tracing redacts PII [arrow-adbc]
Posted by "ryan-syed (via GitHub)" <gi...@apache.org>.
ryan-syed commented on code in PR #3157:
URL: https://github.com/apache/arrow-adbc/pull/3157#discussion_r2208677843
##########
csharp/src/Apache.Arrow.Adbc/Tracing/RedactedValue.cs:
##########
@@ -0,0 +1,94 @@
+/*
+ * 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.Text.Json;
+using System.Text.Json.Serialization;
+
+namespace Apache.Arrow.Adbc.Tracing
+{
+ [JsonConverter(typeof(RedactedValueConverter))]
+ public struct RedactedValue
+ {
+ public const string RedactedValueDefault = "[redacted]";
Review Comment:
curious if the default value would need localization and have to be configurable
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(csharp/src/Apache.Arrow.Adbc): ensure tracing redacts PII [arrow-adbc]
Posted by "birschick-bq (via GitHub)" <gi...@apache.org>.
birschick-bq closed pull request #3157: feat(csharp/src/Apache.Arrow.Adbc): ensure tracing redacts PII
URL: https://github.com/apache/arrow-adbc/pull/3157
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(csharp/src/Apache.Arrow.Adbc): ensure tracing redacts PII [arrow-adbc]
Posted by "davidhcoe (via GitHub)" <gi...@apache.org>.
davidhcoe commented on code in PR #3157:
URL: https://github.com/apache/arrow-adbc/pull/3157#discussion_r2309081879
##########
csharp/src/Drivers/Databricks/Reader/DatabricksReader.cs:
##########
@@ -62,7 +62,7 @@ public DatabricksReader(IHiveServer2Statement statement, Schema schema, IRespons
RecordBatch? next = await this.reader.ReadNextRecordBatchAsync(cancellationToken);
if (next != null)
{
- activity?.AddEvent(SemanticConventions.Messaging.Batch.Response, [new(SemanticConventions.Db.Response.ReturnedRows, next.Length)]);
+ activity?.AddEvent(SemanticConventions.Messaging.Batch.Response, [new(SemanticConventions.Db.Response.ReturnedRows, next.Length)], isPii: false);
Review Comment:
We've traditionally thought of exceptions has containing PII because of the nature of the messages. It could be as mundane as a ArgumentInvalidException to an elaborate `new NewFangledException("david did this")` and the message could be anything. Is there a scenario where this would be set to false?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(csharp/src/Apache.Arrow.Adbc): ensure tracing redacts PII [arrow-adbc]
Posted by "birschick-bq (via GitHub)" <gi...@apache.org>.
birschick-bq commented on code in PR #3157:
URL: https://github.com/apache/arrow-adbc/pull/3157#discussion_r2211077207
##########
csharp/src/Apache.Arrow.Adbc/Tracing/ActivityWithPii.cs:
##########
@@ -0,0 +1,277 @@
+/*
+ * 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 System.Text;
+
+namespace Apache.Arrow.Adbc.Tracing
+{
+ public sealed class ActivityWithPii : IDisposable
+ {
+ private readonly Activity _activity;
+
+ internal ActivityWithPii(Activity activity)
+ {
+ _activity = activity;
+ }
+
+ /// <summary>
+ /// Update the Activity to have baggage with an additional 'key' and value 'value'.
+ /// This shows up in the <see cref="Baggage"/> enumeration as well as the <see cref="GetBaggageItem(string)"/>
+ /// 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 <see cref="Tags"/>.
+ /// Returns 'this' for convenient chaining.
+ /// </summary>
+ /// <returns><see langword="this" /> for convenient chaining.</returns>
+ public ActivityWithPii AddBaggage(string key, RedactedValue value)
+ {
+ _activity.AddBaggage(key, value.ToString());
+ return this;
+ }
+
+ /// <summary>
+ /// Update the Activity to have baggage with an additional 'key' and value 'value'. Use parameter 'isPii' to indicate
+ /// whether the value is PII (Personally Identifiable Information).
+ /// This shows up in the <see cref="Baggage"/> enumeration as well as the <see cref="GetBaggageItem(string)"/>
+ /// 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 <see cref="Tags"/>.
+ /// Returns 'this' for convenient chaining.
+ /// </summary>
+ /// <returns><see langword="this" /> for convenient chaining.</returns>
+ public ActivityWithPii AddBaggage(string key, object? value, bool isPii = true)
+ {
+ _activity.AddBaggage(key, ToRedactedValue(value, isPii).ToString());
+ return this;
+ }
+
+ /// <summary>
+ /// Add <see cref="ActivityEvent" /> object to the <see cref="Events" /> list.
+ /// </summary>
+ /// <param name="e"> object of <see cref="ActivityEvent"/> to add to the attached events list.</param>
+ /// <returns><see langword="this" /> for convenient chaining.</returns>
+ public ActivityWithPii AddEvent(string eventName, IReadOnlyList<KeyValuePair<string, RedactedValue?>>? tags)
+ {
+ ActivityTagsCollection? tagsCollection = ToActivityTagsCollection(tags);
+ _activity.AddEvent(new ActivityEvent(eventName, tags: tagsCollection));
+ return this;
+ }
+
+ /// <summary>
+ /// Add <see cref="ActivityEvent" /> object to the <see cref="Events" /> list. Use parameter 'isPii' to indicate
+ /// whether the tags contain PII (Personally Identifiable Information).
+ /// </summary>
+ /// <param name="e"> object of <see cref="ActivityEvent"/> to add to the attached events list.</param>
+ /// <returns><see langword="this" /> for convenient chaining.</returns>
+ public ActivityWithPii AddEvent(string eventName, IReadOnlyList<KeyValuePair<string, object?>>? tags = default, bool isPii = true)
+ {
+ ActivityTagsCollection? tagsCollection = ToActivityTagsCollection(tags, isPii);
+ _activity.AddEvent(new ActivityEvent(eventName, tags: tagsCollection));
+ return this;
+ }
+
+ /// <summary>
+ /// Add an <see cref="ActivityEvent" /> object containing the exception information to the <see cref="Events" /> list.
+ /// </summary>
+ /// <param name="exception">The exception to add to the attached events list.</param>
+ /// <param name="isPii">Indicates whether the exception message contain PII (Personally Identifiable Information).</param>
+ /// <returns><see langword="this" /> for convenient chaining.</returns>
+ /// <remarks>
+ /// <para>- The name of the event will be "exception", and it will include the tags "exception.message", "exception.stacktrace", and "exception.type".</para>
+ /// <para>- Any registered <see cref="ActivityListener"/> with the <see cref="ActivityListener.ExceptionRecorder"/> callback will be notified about this exception addition
+ /// before the <see cref="ActivityEvent" /> object is added to the <see cref="Events" /> list.</para>
+ /// <para>- Any registered <see cref="ActivityListener"/> with the <see cref="ActivityListener.ExceptionRecorder"/> callback that adds "exception.message", "exception.stacktrace", or "exception.type" tags
+ /// will not have these tags overwritten, except by any subsequent <see cref="ActivityListener"/> that explicitly overwrites them.</para>
+ /// </remarks>
+ public ActivityWithPii AddException(Exception exception, bool isPii = true)
+ {
+ const string ExceptionMessageTag = "exception.message";
+ TagList tagList = new();
+ if (isPii)
+ {
+ tagList.Add(new KeyValuePair<string, object?>(ExceptionMessageTag, new RedactedValue(exception.Message)));
+ }
Review Comment:
It is not clear, but the `AddException` method will add the exception message if it does not exist in the passed `tagList`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(csharp/src/Apache.Arrow.Adbc): ensure tracing redacts PII [arrow-adbc]
Posted by "ryan-syed (via GitHub)" <gi...@apache.org>.
ryan-syed commented on code in PR #3157:
URL: https://github.com/apache/arrow-adbc/pull/3157#discussion_r2208913357
##########
csharp/src/Apache.Arrow.Adbc/Tracing/ActivityWithPii.cs:
##########
@@ -0,0 +1,277 @@
+/*
+ * 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 System.Text;
+
+namespace Apache.Arrow.Adbc.Tracing
+{
+ public sealed class ActivityWithPii : IDisposable
+ {
+ private readonly Activity _activity;
+
+ internal ActivityWithPii(Activity activity)
+ {
+ _activity = activity;
+ }
+
+ /// <summary>
+ /// Update the Activity to have baggage with an additional 'key' and value 'value'.
+ /// This shows up in the <see cref="Baggage"/> enumeration as well as the <see cref="GetBaggageItem(string)"/>
+ /// 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 <see cref="Tags"/>.
+ /// Returns 'this' for convenient chaining.
+ /// </summary>
+ /// <returns><see langword="this" /> for convenient chaining.</returns>
+ public ActivityWithPii AddBaggage(string key, RedactedValue value)
+ {
+ _activity.AddBaggage(key, value.ToString());
+ return this;
+ }
+
+ /// <summary>
+ /// Update the Activity to have baggage with an additional 'key' and value 'value'. Use parameter 'isPii' to indicate
+ /// whether the value is PII (Personally Identifiable Information).
+ /// This shows up in the <see cref="Baggage"/> enumeration as well as the <see cref="GetBaggageItem(string)"/>
+ /// 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 <see cref="Tags"/>.
+ /// Returns 'this' for convenient chaining.
+ /// </summary>
+ /// <returns><see langword="this" /> for convenient chaining.</returns>
+ public ActivityWithPii AddBaggage(string key, object? value, bool isPii = true)
+ {
+ _activity.AddBaggage(key, ToRedactedValue(value, isPii).ToString());
+ return this;
+ }
+
+ /// <summary>
+ /// Add <see cref="ActivityEvent" /> object to the <see cref="Events" /> list.
+ /// </summary>
+ /// <param name="e"> object of <see cref="ActivityEvent"/> to add to the attached events list.</param>
+ /// <returns><see langword="this" /> for convenient chaining.</returns>
+ public ActivityWithPii AddEvent(string eventName, IReadOnlyList<KeyValuePair<string, RedactedValue?>>? tags)
+ {
+ ActivityTagsCollection? tagsCollection = ToActivityTagsCollection(tags);
+ _activity.AddEvent(new ActivityEvent(eventName, tags: tagsCollection));
+ return this;
+ }
+
+ /// <summary>
+ /// Add <see cref="ActivityEvent" /> object to the <see cref="Events" /> list. Use parameter 'isPii' to indicate
+ /// whether the tags contain PII (Personally Identifiable Information).
+ /// </summary>
+ /// <param name="e"> object of <see cref="ActivityEvent"/> to add to the attached events list.</param>
+ /// <returns><see langword="this" /> for convenient chaining.</returns>
+ public ActivityWithPii AddEvent(string eventName, IReadOnlyList<KeyValuePair<string, object?>>? tags = default, bool isPii = true)
+ {
+ ActivityTagsCollection? tagsCollection = ToActivityTagsCollection(tags, isPii);
+ _activity.AddEvent(new ActivityEvent(eventName, tags: tagsCollection));
+ return this;
+ }
+
+ /// <summary>
+ /// Add an <see cref="ActivityEvent" /> object containing the exception information to the <see cref="Events" /> list.
+ /// </summary>
+ /// <param name="exception">The exception to add to the attached events list.</param>
+ /// <param name="isPii">Indicates whether the exception message contain PII (Personally Identifiable Information).</param>
+ /// <returns><see langword="this" /> for convenient chaining.</returns>
+ /// <remarks>
+ /// <para>- The name of the event will be "exception", and it will include the tags "exception.message", "exception.stacktrace", and "exception.type".</para>
+ /// <para>- Any registered <see cref="ActivityListener"/> with the <see cref="ActivityListener.ExceptionRecorder"/> callback will be notified about this exception addition
+ /// before the <see cref="ActivityEvent" /> object is added to the <see cref="Events" /> list.</para>
+ /// <para>- Any registered <see cref="ActivityListener"/> with the <see cref="ActivityListener.ExceptionRecorder"/> callback that adds "exception.message", "exception.stacktrace", or "exception.type" tags
+ /// will not have these tags overwritten, except by any subsequent <see cref="ActivityListener"/> that explicitly overwrites them.</para>
+ /// </remarks>
+ public ActivityWithPii AddException(Exception exception, bool isPii = true)
+ {
+ const string ExceptionMessageTag = "exception.message";
+ TagList tagList = new();
+ if (isPii)
+ {
+ tagList.Add(new KeyValuePair<string, object?>(ExceptionMessageTag, new RedactedValue(exception.Message)));
+ }
+
+ _activity.AddException(exception, tagList);
+ return this;
+ }
+
+ /// <summary>
+ /// Add a new <see cref="ActivityLink"/> to the <see cref="Activity.Links"/> list.
+ /// </summary>
+ /// <param name="traceParent">The traceParent id for the associated <see cref="ActivityContext"/>.</param>
+ /// <param name="tags">The optional list of tags to attach to the event.</param>
+ /// <param name="isPii">Indicates whether the tags contain PII (Personally Identifiable Information).</param>
+ /// <returns><see langword="this"/> for convenient chaining.</returns>
+ public ActivityWithPii AddLink(string traceParent, IReadOnlyList<KeyValuePair<string, object?>>? tags = default, bool isPii = true)
+ {
+ ActivityTagsCollection? tagsCollection = ToActivityTagsCollection(tags, isPii);
+ _activity?.AddLink(new ActivityLink(ActivityContext.Parse(traceParent, null), tags: tagsCollection));
+ return this;
+ }
+
+ /// <summary>
+ /// Update the Activity to have a tag with an additional 'key' and value 'value'.
+ /// This shows up in the <see cref="TagObjects"/> enumeration. It is meant for information that
+ /// is useful to log but not needed for runtime control (for the latter, <see cref="Baggage"/>)
+ /// </summary>
+ /// <returns><see langword="this" /> for convenient chaining.</returns>
+ /// <param name="key">The tag key name</param>
+ /// <param name="value">The tag value mapped to the input key</param>
+ /// <param name="isPii">Indicates whether the value is PII (Personally Identifiable Information).</param>
+ public ActivityWithPii AddTag(string key, object? value, bool isPii = true)
+ {
+ _activity.AddTag(key, ToRedactedValue(value, isPii));
+ return this;
+ }
+
+ /// <summary>
+ /// Update the Activity to have a tag with an additional 'key' and value 'value'.
+ /// This shows up in the <see cref="TagObjects"/> enumeration. It is meant for information that
+ /// is useful to log but not needed for runtime control (for the latter, <see cref="Baggage"/>)
+ /// </summary>
+ /// <returns><see langword="this" /> for convenient chaining.</returns>
+ /// <param name="key">The tag key name</param>
+ /// <param name="value">The tag value mapped to the input key</param>
+ public ActivityWithPii AddTag(string key, RedactedValue value)
+ {
+ _activity.AddTag(key, value);
+ return this;
+ }
+
+ /// <summary>
+ /// Update the Activity to have a tag with an additional 'key' and value 'value'.
+ /// This shows up in the <see cref="TagObjects"/> enumeration. It is meant for information that
+ /// is useful to log but not needed for runtime control (for the latter, <see cref="Baggage"/>)
+ /// </summary>
+ /// <returns><see langword="this" /> for convenient chaining.</returns>
+ /// <param name="key">The tag key name</param>
+ /// <param name="valueFn">The tag value function mapped to the input key</param>
+ /// <param name="isPii">Indicates whether the value is PII (Personally Identifiable Information).</param>
+ public ActivityWithPii AddTag(string key, Func<object?> valueFn, bool isPii = true)
+ {
+ _activity.AddTag(key, ToRedactedValue(valueFn(), isPii));
+ return this;
+ }
+
+ /// <summary>
+ /// Update the Activity to have a tag with an additional 'key' and value 'value'.
+ /// This shows up in the <see cref="TagObjects"/> enumeration. It is meant for information that
+ /// is useful to log but not needed for runtime control (for the latter, <see cref="Baggage"/>)
+ /// </summary>
+ /// <returns><see langword="this" /> for convenient chaining.</returns>
+ /// <param name="key">The tag key name</param>
+ /// <param name="value">The tag value mapped to the input key as a function</param>
+ /// <param name="guidFormat">The format indicator for 16-byte GUID arrays.</param>
+ public ActivityWithPii AddTag(string key, byte[]? value, string? guidFormat, bool isPii = true)
+ {
+ if (value == null)
+ {
+ return AddTag(key, value, isPii);
+ }
+ if (value.Length == 16)
+ {
+ return AddTag(key, new Guid(value).ToString(guidFormat), isPii);
+ }
+ return AddTag(key, ToHexString(value), isPii);
+ }
+
+ /// <summary>
+ /// Update the Activity to have a tag with an additional 'key' and value 'value'.
+ /// This shows up in the <see cref="TagObjects"/> enumeration. It is meant for information that
+ /// is useful to log but not needed for runtime control (for the latter, <see cref="Baggage"/>)
+ /// </summary>
+ /// <returns><see langword="this" /> for convenient chaining.</returns>
+ /// <param name="key">The tag key name as a function</param>
+ /// <param name="value">The tag value mapped to the input key</param>
+ /// /// <param name="condition">The condition to check before adding the tag</param>
+ public ActivityWithPii AddConditionalTag(string key, string? value, bool condition, bool isPii = true)
+ {
+ if (condition)
+ {
+ return AddTag(key, value, isPii);
+ }
+
+ return this;
+ }
+
+ /// <summary>
+ /// Sets the status code and description on the current activity object.
+ /// </summary>
+ /// <param name="code">The status code</param>
+ /// <param name="description">The error status description</param>
+ /// <returns><see langword="this" /> for convenient chaining.</returns>
+ /// <remarks>
+ /// 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.
+ /// </remarks>
+ public ActivityWithPii SetStatus(ActivityStatusCode code, string? description = null)
+ {
+ _activity.SetStatus(code, description);
+ return this;
+ }
+
+ /// <summary>
+ /// Gets status code of the current activity object.
+ /// </summary>
+ public ActivityStatusCode Status => _activity.Status;
+
+ /// <summary>
+ /// Dipsoses the current <see cref="Activity"/> object.
+ /// </summary>
+ public void Dispose()
+ {
+ _activity.Dispose();
+ }
+
+ private static RedactedValue ToRedactedValue(object? value, bool isPii) =>
+ value is RedactedValue redactedValue ? redactedValue : new RedactedValue(value);
Review Comment:
isPii input parameter is not being used while creating the RedactedValue
##########
csharp/src/Apache.Arrow.Adbc/Tracing/RedactedValue.cs:
##########
@@ -0,0 +1,94 @@
+/*
+ * 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.Text.Json;
+using System.Text.Json.Serialization;
+
+namespace Apache.Arrow.Adbc.Tracing
+{
+ [JsonConverter(typeof(RedactedValueConverter))]
+ public struct RedactedValue
+ {
+ public const string RedactedValueDefault = "[redacted]";
Review Comment:
curious if the default value would need localization and have to be configurable
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(csharp/src/Apache.Arrow.Adbc): ensure tracing redacts PII [arrow-adbc]
Posted by "davidhcoe (via GitHub)" <gi...@apache.org>.
davidhcoe commented on code in PR #3157:
URL: https://github.com/apache/arrow-adbc/pull/3157#discussion_r2309061440
##########
csharp/src/Drivers/BigQuery/BigQueryConnection.cs:
##########
@@ -136,30 +136,30 @@ internal BigQueryClient Open(string? projectId = null)
if (projectId.Equals(BigQueryConstants.PublicProjectId, StringComparison.OrdinalIgnoreCase))
{
projectId = BigQueryConstants.DetectProjectId;
- activity?.AddBigQueryTag("change_public_projectId_to_detect_project_id", projectId);
+ activity?.AddBigQueryTag("change_public_projectId_to_detect_project_id", projectId, isPii: false);
}
}
// the billing project can be null if it's not specified
if (this.properties.TryGetValue(BigQueryParameters.BillingProjectId, out billingProjectId))
{
- activity?.AddBigQueryParameterTag((BigQueryParameters.BillingProjectId), billingProjectId);
+ activity?.AddBigQueryParameterTag((BigQueryParameters.BillingProjectId), billingProjectId, isPii: false);
Review Comment:
I could see this potentially being pii, baesd on how people name their projects.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(csharp/src/Apache.Arrow.Adbc): ensure tracing redacts PII [arrow-adbc]
Posted by "birschick-bq (via GitHub)" <gi...@apache.org>.
birschick-bq commented on PR #3157:
URL: https://github.com/apache/arrow-adbc/pull/3157#issuecomment-3085386658
@CurtHagenlocher Let me know if you have any comments or suggestion for this PR, when you have a chance. Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(csharp/src/Apache.Arrow.Adbc): ensure tracing redacts PII [arrow-adbc]
Posted by "birschick-bq (via GitHub)" <gi...@apache.org>.
birschick-bq commented on code in PR #3157:
URL: https://github.com/apache/arrow-adbc/pull/3157#discussion_r2309119604
##########
csharp/src/Drivers/BigQuery/BigQueryConnection.cs:
##########
@@ -136,30 +136,30 @@ internal BigQueryClient Open(string? projectId = null)
if (projectId.Equals(BigQueryConstants.PublicProjectId, StringComparison.OrdinalIgnoreCase))
{
projectId = BigQueryConstants.DetectProjectId;
- activity?.AddBigQueryTag("change_public_projectId_to_detect_project_id", projectId);
+ activity?.AddBigQueryTag("change_public_projectId_to_detect_project_id", projectId, isPii: false);
}
}
// the billing project can be null if it's not specified
if (this.properties.TryGetValue(BigQueryParameters.BillingProjectId, out billingProjectId))
{
- activity?.AddBigQueryParameterTag((BigQueryParameters.BillingProjectId), billingProjectId);
+ activity?.AddBigQueryParameterTag((BigQueryParameters.BillingProjectId), billingProjectId, isPii: false);
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(csharp/src/Apache.Arrow.Adbc): ensure tracing redacts PII [arrow-adbc]
Posted by "davidhcoe (via GitHub)" <gi...@apache.org>.
davidhcoe commented on code in PR #3157:
URL: https://github.com/apache/arrow-adbc/pull/3157#discussion_r2311325239
##########
csharp/src/Apache.Arrow.Adbc/Tracing/ActivityTrace.cs:
##########
@@ -75,17 +79,18 @@ public ActivityTrace(string? activitySourceName = default, string? activitySourc
/// status is set to <see cref="ActivityStatusCode.Error"/> and an Activity <see cref="ActivityEvent"/> is added to the activity
/// and finally the exception is rethrown.
/// </remarks>
- public void TraceActivity(Action<Activity?> call, [CallerMemberName] string? activityName = default, string? traceParent = default)
+ public void TraceActivity(Action<ActivityWithPii?> call, [CallerMemberName] string? activityName = default, string? traceParent = default, bool exceptionIsPii = true)
Review Comment:
I think we can remove bool exceptionIsPii = true now too, correct?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(csharp/src/Apache.Arrow.Adbc): ensure tracing redacts PII [arrow-adbc]
Posted by "birschick-bq (via GitHub)" <gi...@apache.org>.
birschick-bq commented on PR #3157:
URL: https://github.com/apache/arrow-adbc/pull/3157#issuecomment-3104102197
@CurtHagenlocher Just checking to see if you have any comments or suggestions for this PR. Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(csharp/src/Apache.Arrow.Adbc): ensure tracing redacts PII [arrow-adbc]
Posted by "birschick-bq (via GitHub)" <gi...@apache.org>.
birschick-bq commented on code in PR #3157:
URL: https://github.com/apache/arrow-adbc/pull/3157#discussion_r2309121295
##########
csharp/src/Drivers/Databricks/Reader/DatabricksReader.cs:
##########
@@ -62,7 +62,7 @@ public DatabricksReader(IHiveServer2Statement statement, Schema schema, IRespons
RecordBatch? next = await this.reader.ReadNextRecordBatchAsync(cancellationToken);
if (next != null)
{
- activity?.AddEvent(SemanticConventions.Messaging.Batch.Response, [new(SemanticConventions.Db.Response.ReturnedRows, next.Length)]);
+ activity?.AddEvent(SemanticConventions.Messaging.Batch.Response, [new(SemanticConventions.Db.Response.ReturnedRows, next.Length)], isPii: false);
Review Comment:
I've removed all the `exceptionIsPii: false` for all occurrences. Do you think I should remove the optional parameter, altogether?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(csharp/src/Apache.Arrow.Adbc): ensure tracing redacts PII [arrow-adbc]
Posted by "birschick-bq (via GitHub)" <gi...@apache.org>.
birschick-bq commented on code in PR #3157:
URL: https://github.com/apache/arrow-adbc/pull/3157#discussion_r2311348914
##########
csharp/src/Apache.Arrow.Adbc/Tracing/ActivityTrace.cs:
##########
@@ -75,17 +79,18 @@ public ActivityTrace(string? activitySourceName = default, string? activitySourc
/// status is set to <see cref="ActivityStatusCode.Error"/> and an Activity <see cref="ActivityEvent"/> is added to the activity
/// and finally the exception is rethrown.
/// </remarks>
- public void TraceActivity(Action<Activity?> call, [CallerMemberName] string? activityName = default, string? traceParent = default)
+ public void TraceActivity(Action<ActivityWithPii?> call, [CallerMemberName] string? activityName = default, string? traceParent = default, bool exceptionIsPii = true)
Review Comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(csharp/src/Apache.Arrow.Adbc): ensure tracing redacts PII [arrow-adbc]
Posted by "birschick-bq (via GitHub)" <gi...@apache.org>.
birschick-bq commented on PR #3157:
URL: https://github.com/apache/arrow-adbc/pull/3157#issuecomment-3141264703
@CurtHagenlocher This contains binary breaking changes. Let me know if you want me to refactor to only add new and preserve the existing implementation. Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(csharp/src/Apache.Arrow.Adbc): ensure tracing redacts PII [arrow-adbc]
Posted by "birschick-bq (via GitHub)" <gi...@apache.org>.
birschick-bq commented on code in PR #3157:
URL: https://github.com/apache/arrow-adbc/pull/3157#discussion_r2274652494
##########
csharp/src/Apache.Arrow.Adbc/Tracing/ActivityWithPii.cs:
##########
@@ -0,0 +1,225 @@
+/*
+ * 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 System.Text;
+
+namespace Apache.Arrow.Adbc.Tracing
+{
+ /// <summary>
+ /// ActivityWithPii is a wrapper to the <see cref="Activity "/> which represents operation with
+ /// context to be used for logging.
+ ///
+ /// By default, all values added the ActivityWithPii are considered to be PII (Personally Identifiable Information).
+ ///
+ /// To indicate that a value is not PII, you can use the `isPii` parameter in methods.
+ /// </summary>
+ public sealed class ActivityWithPii : IDisposable
+ {
+ private readonly Activity _activity;
+
+ internal ActivityWithPii(Activity activity)
+ {
+ _activity = activity;
+ }
+
+ /// <summary>
+ /// Update the Activity to have baggage with an additional 'key' and value 'value'. Use parameter 'isPii' to indicate
+ /// whether the value is PII (Personally Identifiable Information).
+ /// This shows up in the <see cref="Baggage"/> enumeration as well as the <see cref="GetBaggageItem(string)"/>
+ /// 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 <see cref="Tags"/>.
+ /// Returns 'this' for convenient chaining.
+ /// </summary>
+ /// <returns><see langword="this" /> for convenient chaining.</returns>
+ public ActivityWithPii AddBaggage(string key, string? value, bool isPii = true)
+ {
+ object? obj = MaybeToRedactedValue(value, isPii);
+ _activity.AddBaggage(key, obj?.ToString());
+ return this;
+ }
+
+ /// <summary>
+ /// Add <see cref="ActivityEvent" /> object to the <see cref="Events" /> list. Use parameter 'isPii' to indicate
+ /// whether the tags contain PII (Personally Identifiable Information).
+ /// </summary>
+ /// <param name="e"> object of <see cref="ActivityEvent"/> to add to the attached events list.</param>
+ /// <returns><see langword="this" /> for convenient chaining.</returns>
+ public ActivityWithPii AddEvent(string eventName, IReadOnlyList<KeyValuePair<string, object?>>? tags = default, bool isPii = true)
+ {
+ ActivityTagsCollection? tagsCollection = ToActivityTagsCollection(tags, isPii);
+ _activity.AddEvent(new ActivityEvent(eventName, tags: tagsCollection));
+ return this;
+ }
+
+ /// <summary>
+ /// Add an <see cref="ActivityEvent" /> object containing the exception information to the <see cref="Events" /> list.
+ /// </summary>
+ /// <param name="exception">The exception to add to the attached events list.</param>
+ /// <param name="isPii">Indicates whether the exception message contain PII (Personally Identifiable Information).</param>
+ /// <returns><see langword="this" /> for convenient chaining.</returns>
+ /// <remarks>
+ /// <para>- The name of the event will be "exception", and it will include the tags "exception.message", "exception.stacktrace", and "exception.type".</para>
+ /// <para>- Any registered <see cref="ActivityListener"/> with the <see cref="ActivityListener.ExceptionRecorder"/> callback will be notified about this exception addition
+ /// before the <see cref="ActivityEvent" /> object is added to the <see cref="Events" /> list.</para>
+ /// <para>- Any registered <see cref="ActivityListener"/> with the <see cref="ActivityListener.ExceptionRecorder"/> callback that adds "exception.message", "exception.stacktrace", or "exception.type" tags
+ /// will not have these tags overwritten, except by any subsequent <see cref="ActivityListener"/> that explicitly overwrites them.</para>
+ /// </remarks>
+ public ActivityWithPii AddException(Exception exception, bool isPii = true)
+ {
+ const string ExceptionMessageTag = "exception.message";
+ TagList tagList = new();
+ if (isPii)
+ {
+ tagList.Add(new KeyValuePair<string, object?>(ExceptionMessageTag, new RedactedValue(exception.Message)));
+ }
+
+ _activity.AddException(exception, tagList);
+ return this;
+ }
+
+ /// <summary>
+ /// Add a new <see cref="ActivityLink"/> to the <see cref="Activity.Links"/> list.
+ /// </summary>
+ /// <param name="traceParent">The traceParent id for the associated <see cref="ActivityContext"/>.</param>
+ /// <param name="tags">The optional list of tags to attach to the event.</param>
+ /// <param name="isPii">Indicates whether the tags contain PII (Personally Identifiable Information).</param>
+ /// <returns><see langword="this"/> for convenient chaining.</returns>
+ public ActivityWithPii AddLink(string traceParent, IReadOnlyList<KeyValuePair<string, object?>>? tags = default, bool isPii = true)
+ {
+ ActivityTagsCollection? tagsCollection = ToActivityTagsCollection(tags, isPii);
+ _activity?.AddLink(new ActivityLink(ActivityContext.Parse(traceParent, null), tags: tagsCollection));
+ return this;
+ }
+
+ /// <summary>
+ /// Update the Activity to have a tag with an additional 'key' and value 'value'.
+ /// This shows up in the <see cref="TagObjects"/> enumeration. It is meant for information that
+ /// is useful to log but not needed for runtime control (for the latter, <see cref="Baggage"/>)
+ /// </summary>
+ /// <returns><see langword="this" /> for convenient chaining.</returns>
+ /// <param name="key">The tag key name</param>
+ /// <param name="value">The tag value mapped to the input key</param>
+ /// <param name="isPii">Indicates whether the value is PII (Personally Identifiable Information).</param>
+ public ActivityWithPii AddTag(string key, object? value, bool isPii = true)
+ {
+ value = MaybeToRedactedValue(value, isPii);
+ _activity.AddTag(key, value);
+ return this;
+ }
+
+ /// <summary>
+ /// Update the Activity to have a tag with an additional 'key' and value 'value'.
+ /// This shows up in the <see cref="TagObjects"/> enumeration. It is meant for information that
+ /// is useful to log but not needed for runtime control (for the latter, <see cref="Baggage"/>)
+ /// </summary>
+ /// <returns><see langword="this" /> for convenient chaining.</returns>
+ /// <param name="key">The tag key name</param>
+ /// <param name="valueFn">The tag value function mapped to the input key</param>
+ /// <param name="isPii">Indicates whether the value is PII (Personally Identifiable Information).</param>
+ public ActivityWithPii AddTag(string key, Func<object?> valueFn, bool isPii = true)
+ {
+ object? value = MaybeToRedactedValue(valueFn(), isPii);
+ _activity.AddTag(key, value);
+ return this;
+ }
+
+ /// <summary>
+ /// Update the Activity to have a tag with an additional 'key' and value 'value'.
+ /// This shows up in the <see cref="TagObjects"/> enumeration. It is meant for information that
+ /// is useful to log but not needed for runtime control (for the latter, <see cref="Baggage"/>)
+ /// </summary>
+ /// <returns><see langword="this" /> for convenient chaining.</returns>
+ /// <param name="key">The tag key name</param>
+ /// <param name="value">The tag value mapped to the input key as a function</param>
+ /// <param name="guidFormat">The format indicator for 16-byte GUID arrays.</param>
+ public ActivityWithPii AddTag(string key, byte[]? value, string? guidFormat, bool isPii = true)
+ {
+ if (value == null)
+ {
+ return AddTag(key, value, isPii);
+ }
+ if (value.Length == 16)
Review Comment:
Thanks. Addressed. The Guid documentation indicates it will throw an error if the byte array is not exactly 16 bytes in length.
https://learn.microsoft.com/en-us/dotnet/api/system.guid.-ctor?view=net-9.0#system-guid-ctor(system-byte())
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(csharp/src/Apache.Arrow.Adbc): ensure tracing redacts PII [arrow-adbc]
Posted by "jduo (via GitHub)" <gi...@apache.org>.
jduo commented on code in PR #3157:
URL: https://github.com/apache/arrow-adbc/pull/3157#discussion_r2267895118
##########
csharp/src/Apache.Arrow.Adbc/Tracing/ActivityWithPii.cs:
##########
@@ -0,0 +1,225 @@
+/*
+ * 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 System.Text;
+
+namespace Apache.Arrow.Adbc.Tracing
+{
+ /// <summary>
+ /// ActivityWithPii is a wrapper to the <see cref="Activity "/> which represents operation with
+ /// context to be used for logging.
+ ///
+ /// By default, all values added the ActivityWithPii are considered to be PII (Personally Identifiable Information).
+ ///
+ /// To indicate that a value is not PII, you can use the `isPii` parameter in methods.
+ /// </summary>
+ public sealed class ActivityWithPii : IDisposable
+ {
+ private readonly Activity _activity;
+
+ internal ActivityWithPii(Activity activity)
+ {
+ _activity = activity;
+ }
+
+ /// <summary>
+ /// Update the Activity to have baggage with an additional 'key' and value 'value'. Use parameter 'isPii' to indicate
+ /// whether the value is PII (Personally Identifiable Information).
+ /// This shows up in the <see cref="Baggage"/> enumeration as well as the <see cref="GetBaggageItem(string)"/>
+ /// 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 <see cref="Tags"/>.
+ /// Returns 'this' for convenient chaining.
+ /// </summary>
+ /// <returns><see langword="this" /> for convenient chaining.</returns>
+ public ActivityWithPii AddBaggage(string key, string? value, bool isPii = true)
+ {
+ object? obj = MaybeToRedactedValue(value, isPii);
+ _activity.AddBaggage(key, obj?.ToString());
+ return this;
+ }
+
+ /// <summary>
+ /// Add <see cref="ActivityEvent" /> object to the <see cref="Events" /> list. Use parameter 'isPii' to indicate
+ /// whether the tags contain PII (Personally Identifiable Information).
+ /// </summary>
+ /// <param name="e"> object of <see cref="ActivityEvent"/> to add to the attached events list.</param>
+ /// <returns><see langword="this" /> for convenient chaining.</returns>
+ public ActivityWithPii AddEvent(string eventName, IReadOnlyList<KeyValuePair<string, object?>>? tags = default, bool isPii = true)
+ {
+ ActivityTagsCollection? tagsCollection = ToActivityTagsCollection(tags, isPii);
+ _activity.AddEvent(new ActivityEvent(eventName, tags: tagsCollection));
+ return this;
+ }
+
+ /// <summary>
+ /// Add an <see cref="ActivityEvent" /> object containing the exception information to the <see cref="Events" /> list.
+ /// </summary>
+ /// <param name="exception">The exception to add to the attached events list.</param>
+ /// <param name="isPii">Indicates whether the exception message contain PII (Personally Identifiable Information).</param>
+ /// <returns><see langword="this" /> for convenient chaining.</returns>
+ /// <remarks>
+ /// <para>- The name of the event will be "exception", and it will include the tags "exception.message", "exception.stacktrace", and "exception.type".</para>
+ /// <para>- Any registered <see cref="ActivityListener"/> with the <see cref="ActivityListener.ExceptionRecorder"/> callback will be notified about this exception addition
+ /// before the <see cref="ActivityEvent" /> object is added to the <see cref="Events" /> list.</para>
+ /// <para>- Any registered <see cref="ActivityListener"/> with the <see cref="ActivityListener.ExceptionRecorder"/> callback that adds "exception.message", "exception.stacktrace", or "exception.type" tags
+ /// will not have these tags overwritten, except by any subsequent <see cref="ActivityListener"/> that explicitly overwrites them.</para>
+ /// </remarks>
+ public ActivityWithPii AddException(Exception exception, bool isPii = true)
+ {
+ const string ExceptionMessageTag = "exception.message";
+ TagList tagList = new();
+ if (isPii)
+ {
+ tagList.Add(new KeyValuePair<string, object?>(ExceptionMessageTag, new RedactedValue(exception.Message)));
+ }
+
+ _activity.AddException(exception, tagList);
+ return this;
+ }
+
+ /// <summary>
+ /// Add a new <see cref="ActivityLink"/> to the <see cref="Activity.Links"/> list.
+ /// </summary>
+ /// <param name="traceParent">The traceParent id for the associated <see cref="ActivityContext"/>.</param>
+ /// <param name="tags">The optional list of tags to attach to the event.</param>
+ /// <param name="isPii">Indicates whether the tags contain PII (Personally Identifiable Information).</param>
+ /// <returns><see langword="this"/> for convenient chaining.</returns>
+ public ActivityWithPii AddLink(string traceParent, IReadOnlyList<KeyValuePair<string, object?>>? tags = default, bool isPii = true)
+ {
+ ActivityTagsCollection? tagsCollection = ToActivityTagsCollection(tags, isPii);
+ _activity?.AddLink(new ActivityLink(ActivityContext.Parse(traceParent, null), tags: tagsCollection));
+ return this;
+ }
+
+ /// <summary>
+ /// Update the Activity to have a tag with an additional 'key' and value 'value'.
+ /// This shows up in the <see cref="TagObjects"/> enumeration. It is meant for information that
+ /// is useful to log but not needed for runtime control (for the latter, <see cref="Baggage"/>)
+ /// </summary>
+ /// <returns><see langword="this" /> for convenient chaining.</returns>
+ /// <param name="key">The tag key name</param>
+ /// <param name="value">The tag value mapped to the input key</param>
+ /// <param name="isPii">Indicates whether the value is PII (Personally Identifiable Information).</param>
+ public ActivityWithPii AddTag(string key, object? value, bool isPii = true)
+ {
+ value = MaybeToRedactedValue(value, isPii);
+ _activity.AddTag(key, value);
+ return this;
+ }
+
+ /// <summary>
+ /// Update the Activity to have a tag with an additional 'key' and value 'value'.
+ /// This shows up in the <see cref="TagObjects"/> enumeration. It is meant for information that
+ /// is useful to log but not needed for runtime control (for the latter, <see cref="Baggage"/>)
+ /// </summary>
+ /// <returns><see langword="this" /> for convenient chaining.</returns>
+ /// <param name="key">The tag key name</param>
+ /// <param name="valueFn">The tag value function mapped to the input key</param>
+ /// <param name="isPii">Indicates whether the value is PII (Personally Identifiable Information).</param>
+ public ActivityWithPii AddTag(string key, Func<object?> valueFn, bool isPii = true)
+ {
+ object? value = MaybeToRedactedValue(valueFn(), isPii);
+ _activity.AddTag(key, value);
+ return this;
+ }
+
+ /// <summary>
+ /// Update the Activity to have a tag with an additional 'key' and value 'value'.
+ /// This shows up in the <see cref="TagObjects"/> enumeration. It is meant for information that
+ /// is useful to log but not needed for runtime control (for the latter, <see cref="Baggage"/>)
+ /// </summary>
+ /// <returns><see langword="this" /> for convenient chaining.</returns>
+ /// <param name="key">The tag key name</param>
+ /// <param name="value">The tag value mapped to the input key as a function</param>
+ /// <param name="guidFormat">The format indicator for 16-byte GUID arrays.</param>
+ public ActivityWithPii AddTag(string key, byte[]? value, string? guidFormat, bool isPii = true)
+ {
+ if (value == null)
+ {
+ return AddTag(key, value, isPii);
+ }
+ if (value.Length == 16)
Review Comment:
This should check if guidFormat is non-null? It is strange that having a byte array of length 16 is automatically a GUID.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(csharp/src/Apache.Arrow.Adbc): ensure tracing redacts PII [arrow-adbc]
Posted by "ryan-syed (via GitHub)" <gi...@apache.org>.
ryan-syed commented on code in PR #3157:
URL: https://github.com/apache/arrow-adbc/pull/3157#discussion_r2208933589
##########
csharp/src/Apache.Arrow.Adbc/Tracing/ActivityWithPii.cs:
##########
@@ -0,0 +1,277 @@
+/*
+ * 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 System.Text;
+
+namespace Apache.Arrow.Adbc.Tracing
+{
+ public sealed class ActivityWithPii : IDisposable
+ {
+ private readonly Activity _activity;
+
+ internal ActivityWithPii(Activity activity)
+ {
+ _activity = activity;
+ }
+
+ /// <summary>
+ /// Update the Activity to have baggage with an additional 'key' and value 'value'.
+ /// This shows up in the <see cref="Baggage"/> enumeration as well as the <see cref="GetBaggageItem(string)"/>
+ /// 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 <see cref="Tags"/>.
+ /// Returns 'this' for convenient chaining.
+ /// </summary>
+ /// <returns><see langword="this" /> for convenient chaining.</returns>
+ public ActivityWithPii AddBaggage(string key, RedactedValue value)
+ {
+ _activity.AddBaggage(key, value.ToString());
+ return this;
+ }
+
+ /// <summary>
+ /// Update the Activity to have baggage with an additional 'key' and value 'value'. Use parameter 'isPii' to indicate
+ /// whether the value is PII (Personally Identifiable Information).
+ /// This shows up in the <see cref="Baggage"/> enumeration as well as the <see cref="GetBaggageItem(string)"/>
+ /// 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 <see cref="Tags"/>.
+ /// Returns 'this' for convenient chaining.
+ /// </summary>
+ /// <returns><see langword="this" /> for convenient chaining.</returns>
+ public ActivityWithPii AddBaggage(string key, object? value, bool isPii = true)
+ {
+ _activity.AddBaggage(key, ToRedactedValue(value, isPii).ToString());
+ return this;
+ }
+
+ /// <summary>
+ /// Add <see cref="ActivityEvent" /> object to the <see cref="Events" /> list.
+ /// </summary>
+ /// <param name="e"> object of <see cref="ActivityEvent"/> to add to the attached events list.</param>
+ /// <returns><see langword="this" /> for convenient chaining.</returns>
+ public ActivityWithPii AddEvent(string eventName, IReadOnlyList<KeyValuePair<string, RedactedValue?>>? tags)
+ {
+ ActivityTagsCollection? tagsCollection = ToActivityTagsCollection(tags);
+ _activity.AddEvent(new ActivityEvent(eventName, tags: tagsCollection));
+ return this;
+ }
+
+ /// <summary>
+ /// Add <see cref="ActivityEvent" /> object to the <see cref="Events" /> list. Use parameter 'isPii' to indicate
+ /// whether the tags contain PII (Personally Identifiable Information).
+ /// </summary>
+ /// <param name="e"> object of <see cref="ActivityEvent"/> to add to the attached events list.</param>
+ /// <returns><see langword="this" /> for convenient chaining.</returns>
+ public ActivityWithPii AddEvent(string eventName, IReadOnlyList<KeyValuePair<string, object?>>? tags = default, bool isPii = true)
+ {
+ ActivityTagsCollection? tagsCollection = ToActivityTagsCollection(tags, isPii);
+ _activity.AddEvent(new ActivityEvent(eventName, tags: tagsCollection));
+ return this;
+ }
+
+ /// <summary>
+ /// Add an <see cref="ActivityEvent" /> object containing the exception information to the <see cref="Events" /> list.
+ /// </summary>
+ /// <param name="exception">The exception to add to the attached events list.</param>
+ /// <param name="isPii">Indicates whether the exception message contain PII (Personally Identifiable Information).</param>
+ /// <returns><see langword="this" /> for convenient chaining.</returns>
+ /// <remarks>
+ /// <para>- The name of the event will be "exception", and it will include the tags "exception.message", "exception.stacktrace", and "exception.type".</para>
+ /// <para>- Any registered <see cref="ActivityListener"/> with the <see cref="ActivityListener.ExceptionRecorder"/> callback will be notified about this exception addition
+ /// before the <see cref="ActivityEvent" /> object is added to the <see cref="Events" /> list.</para>
+ /// <para>- Any registered <see cref="ActivityListener"/> with the <see cref="ActivityListener.ExceptionRecorder"/> callback that adds "exception.message", "exception.stacktrace", or "exception.type" tags
+ /// will not have these tags overwritten, except by any subsequent <see cref="ActivityListener"/> that explicitly overwrites them.</para>
+ /// </remarks>
+ public ActivityWithPii AddException(Exception exception, bool isPii = true)
+ {
+ const string ExceptionMessageTag = "exception.message";
+ TagList tagList = new();
+ if (isPii)
+ {
+ tagList.Add(new KeyValuePair<string, object?>(ExceptionMessageTag, new RedactedValue(exception.Message)));
+ }
Review Comment:
if there is no pii, should we not still add the exception message without redacting it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org