Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -7,34 +7,45 @@

namespace OpenTelemetry.Exporter.OneCollector;

public enum EventNameDelimiter
{
Period,
Underscore
}

internal sealed class EventNameManager
{
// Note: OneCollector will silently drop events which have a name less than 4 characters.
internal const int MinimumEventFullNameLength = 4;
internal const int MaximumEventFullNameLength = 100;

// Make change to allow underscore in eventnamespace below.
private static readonly Regex EventNamespaceValidationRegex = new(@"^[A-Za-z](?:\.?[A-Za-z0-9]+?)*$", RegexOptions.Compiled);
private static readonly Regex EventNameValidationRegex = new(@"^[A-Za-z][A-Za-z0-9]*$", RegexOptions.Compiled);

private readonly string defaultEventNamespace;
private readonly string defaultEventName;
private readonly EventNameDelimiter defaultEventNameDelimiter;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should just be 'eventNameDelimiter'.
The 'defaultEventNamespace' and 'defaultEventName' properties are about the values for a default event, not the default values for an arbitrary event

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is - someone could call log(dataFields: foo) without passing an event name, and the event name would be the default event name. (pseudocode, something like that)

private readonly IReadOnlyDictionary<string, EventFullName>? eventFullNameMappings;
private readonly ResolvedEventFullName defaultEventFullName;
private readonly Hashtable eventNamespaceCache = new(StringComparer.OrdinalIgnoreCase);

public EventNameManager(
string defaultEventNamespace,
string defaultEventName,
IReadOnlyDictionary<string, EventFullName>? eventFullNameMappings = null)
IReadOnlyDictionary<string, EventFullName>? eventFullNameMappings = null,
EventNameDelimiter eventNameDelimiter = EventNameDelimiter.Period)
{
Debug.Assert(defaultEventNamespace != null, "defaultEventNamespace was null");
Debug.Assert(defaultEventName != null, "defaultEventName was null");

this.defaultEventNamespace = defaultEventNamespace!;
this.defaultEventName = defaultEventName!;
this.eventFullNameMappings = eventFullNameMappings;
this.defaultEventNameDelimiter = eventNameDelimiter;

this.defaultEventFullName = new(
eventFullName: BuildEventFullName(this.defaultEventNamespace, this.defaultEventName),
eventFullName: BuildEventFullName(this.defaultEventNamespace, this.defaultEventName, this.defaultEventNameDelimiter),
originalEventNamespace: null,
originalEventName: null);

Expand All @@ -54,7 +65,8 @@ public static bool IsEventNameValid(string eventName)

public ResolvedEventFullName ResolveEventFullName(
string? eventNamespace,
string? eventName)
string? eventName,
EventNameDelimiter eventNameDelimiter = EventNameDelimiter.Period)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this do anything?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, not needed. Will remove.

{
var originalEventNamespace = eventNamespace;
var originalEventName = eventName;
Expand Down Expand Up @@ -112,7 +124,7 @@ public ResolvedEventFullName ResolveEventFullName(
return resolvedEventFullName;
}

private static byte[] BuildEventFullName(string eventNamespace, string eventName)
private static byte[] BuildEventFullName(string eventNamespace, string eventName, EventNameDelimiter eventNameDelimiter = EventNameDelimiter.Period)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably by the time this function is reached, the eventNameDelimiter should be a byte and not an enum

Copy link

@gesiu gesiu Oct 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, unless we really want this to be settable on a per-event basis, this shouldn't be a parameter - you should just reference the property of the EventNameManager

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed.

{
Span<byte> destination = stackalloc byte[128];

Expand All @@ -124,7 +136,8 @@ private static byte[] BuildEventFullName(string eventNamespace, string eventName
{
WriteEventFullNameComponent(eventNamespace, destination, ref cursor);

destination[cursor++] = (byte)'.';
byte delimiter = eventNameDelimiter == EventNameDelimiter.Period ? (byte)'.' : (byte)'_';
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should use a lookup table

destination[cursor++] = delimiter;
}

WriteEventFullNameComponent(eventName, destination, ref cursor);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,9 @@ public sealed class OneCollectorLogExporterOptions : OneCollectorExporterOptions
/// </item>
/// </list>
/// </remarks>

public EventNameDelimiter DefaultEventNameDelimiter { get; set; } = EventNameDelimiter.Period;

public IReadOnlyDictionary<string, string>? EventFullNameMappings
{
get => this.eventFullNameMappings;
Expand Down
Loading