Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Instrumentation.EventCounters] Stabilize and Enable Testing #620

Merged
merged 36 commits into from
Oct 13, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
57986a6
wip
mic-max Sep 1, 2022
51d9cce
nit
mic-max Sep 1, 2022
59e4faa
Rename options class
mic-max Sep 1, 2022
2e2af13
skip failing test
mic-max Sep 1, 2022
8fbb353
fix build error
mic-max Sep 1, 2022
e13c37f
whitespacing
mic-max Sep 1, 2022
3eb1061
Update EventCounterListenerTests.cs
mic-max Sep 6, 2022
97ed727
Add changelog, owner
mic-max Sep 6, 2022
732bc59
clean
mic-max Sep 6, 2022
a1566d1
readme fixup
mic-max Sep 6, 2022
1f5b50e
Merge branch 'main' into eventcounters-stabilize
mic-max Sep 6, 2022
60938d3
use same naming scheme as other instrumentations
mic-max Sep 6, 2022
adca7c5
Merge branch 'eventcounters-stabilize' of https://github.com/mic-max/…
mic-max Sep 6, 2022
37414ca
Update MeterProviderBuilderExtensions.cs
mic-max Sep 7, 2022
1a0c496
Set default polling time to 1 second
mic-max Sep 13, 2022
906b84c
Update README.md
mic-max Sep 13, 2022
d5c7fdf
AddCounters and ShouldListenTo methods
mic-max Sep 13, 2022
8d0a30e
Rename EventCountersMetricsTests file
mic-max Sep 14, 2022
c8af401
Update EventCountersMetricsTests.cs
mic-max Sep 14, 2022
99097fc
Update EventCountersMetrics.cs
mic-max Sep 14, 2022
78d981d
Throw error when registering multiple times
mic-max Sep 14, 2022
f73858b
Revert "Throw error when registering multiple times"
mic-max Sep 14, 2022
b44f952
privify EnableEventsArguments
mic-max Sep 22, 2022
1ef006e
Merge branch 'main' into eventcounters-stabilize
mic-max Sep 22, 2022
12cbb30
pr comments
mic-max Sep 22, 2022
ecaeea4
Merge branch 'eventcounters-stabilize' of https://github.com/mic-max/…
mic-max Sep 22, 2022
9398617
tada
mic-max Sep 28, 2022
e558609
Update EventCountersInstrumentationOptions.cs
mic-max Sep 28, 2022
72e52d1
exception tests
mic-max Sep 29, 2022
54d8b2c
fixing recommendations
mic-max Oct 5, 2022
6daa63d
cleanup
mic-max Oct 6, 2022
553ae87
fix
mic-max Oct 6, 2022
556b65e
CI rerun
mic-max Oct 10, 2022
71560ed
remove duplicate checks sdk already does
mic-max Oct 13, 2022
65521ee
Merge branch 'main' into eventcounters-stabilize
utpilla Oct 13, 2022
a6bfef2
Merge branch 'main' into eventcounters-stabilize
utpilla Oct 13, 2022
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
2 changes: 2 additions & 0 deletions .github/component_owners.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ components:
- ejsmith
src/OpenTelemetry.Instrumentation.EventCounters/:
- hananiel
- mic-max
src/OpenTelemetry.Instrumentation.GrpcCore/:
- pcwiese
src/OpenTelemetry.Instrumentation.Hangfire/:
Expand Down Expand Up @@ -99,6 +100,7 @@ components:
- ejsmith
test/OpenTelemetry.Instrumentation.EventCounters.Tests/:
- hananiel
- mic-max
test/OpenTelemetry.Instrumentation.GrpcCore.Tests/:
- pcwiese
test/OpenTelemetry.Instrumentation.Hangfire.Tests/:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## Unreleased

* Simplified implementation. EventSources must be explicitly configured to be
listened to now.
([#620](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/620))

## 0.1.0-alpha.1

Released 2022-Jul-12
Expand Down

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -22,25 +22,37 @@ namespace OpenTelemetry.Instrumentation.EventCounters;
/// EventSource events emitted from the project.
/// </summary>
[EventSource(Name = "OpenTelemetry-Instrumentation-EventCounters")]
internal class EventCountersInstrumentationEventSource : EventSource
internal sealed class EventCountersInstrumentationEventSource : EventSource
{
public static readonly EventCountersInstrumentationEventSource Log = new();

[Event(1, Message = "Error occurred while processing eventCounter, EventCounter: {0}, Exception: {2}", Level = EventLevel.Error)]
public void ErrorEventCounter(string counterName, string exception)
[Event(1, Level = EventLevel.Warning, Message = "Error while writing event from source: {0} - {1}.")]
internal void ErrorWhileWritingEvent(string eventSourceName, string exceptionMessage)
{
this.WriteEvent(1, counterName, exception);
this.WriteEvent(1, eventSourceName, exceptionMessage);
}

[Event(2, Level = EventLevel.Warning, Message = @"Ignoring event written from EventSource: {0} as payload is not IDictionary to extract metrics.")]
public void IgnoreEventWrittenAsEventPayloadNotParseable(string eventSourceName)
[Event(2, Level = EventLevel.Warning, Message = "Event data payload not parseable from source: {0}.")]
internal void IgnoreEventWrittenEventArgsPayloadNotParseable(string eventSourceName)
{
this.WriteEvent(2, eventSourceName);
}

[Event(3, Level = EventLevel.Warning, Message = "Event data has no name from source: {0}.")]
internal void IgnoreEventWrittenEventArgsWithoutName(string eventSourceName)
{
this.WriteEvent(3, eventSourceName);
}

[Event(4, Level = EventLevel.Warning, Message = "Event data payload problem with values of Mean, Increment from source: {0}.")]
internal void IgnoreMeanIncrementConflict(string eventSourceName)
{
this.WriteEvent(4, eventSourceName);
}

[Event(3, Level = EventLevel.Warning, Message = @"EventCountersInstrumentation - {0} failed with exception: {1}.")]
public void EventCountersInstrumentationWarning(string stage, string exceptionMessage)
[Event(5, Level = EventLevel.Warning, Message = "Event data has name other than 'EventCounters' from source: {0}.")]
internal void IgnoreNonEventCountersName(string eventSourceName)
{
this.WriteEvent(8, stage, exceptionMessage);
this.WriteEvent(5, eventSourceName);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
// <copyright file="EventCountersInstrumentationOptions.cs" company="OpenTelemetry Authors">
// Copyright The OpenTelemetry Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
// </copyright>

using System;
using System.Collections.Generic;
using System.Linq;

namespace OpenTelemetry.Instrumentation.EventCounters;

/// <summary>
/// EventCounterListener Options.
/// </summary>
public class EventCountersInstrumentationOptions
{
private readonly HashSet<string> eventSourceNames = new();

/// <summary>
/// Gets or sets the subscription interval in seconds for reading values
/// from the configured EventCounters.
/// </summary>
public int RefreshIntervalSecs { get; set; } = 1;
mic-max marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Listens to EventCounters from the given EventSource name.
mic-max marked this conversation as resolved.
Show resolved Hide resolved
/// </summary>
/// <param name="names">The EventSource names to listen to.</param>
public void AddEventSources(params string[] names)
{
if (names.Contains("System.Runtime"))
{
throw new NotSupportedException("Use the `OpenTelemetry.Instrumentation.Runtime` or `OpenTelemetry.Instrumentation.Process` instrumentations.");
}

this.eventSourceNames.UnionWith(names);
}

/// <summary>
/// Returns whether or not an EventSource should be enabled on the EventListener.
/// </summary>
/// <param name="eventSourceName">The EventSource name.</param>
/// <returns><c>true</c> when an EventSource with the name <paramref name="eventSourceName"/> should be enabled.</returns>
internal bool ShouldListenToSource(string eventSourceName)
Copy link
Contributor

Choose a reason for hiding this comment

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

This method feels like it should belong to EventCountersMetrics class.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Then the eventSouceNames HashSet would also move and then this class would be empty. I'll keep here but I kinda see your point

Copy link
Contributor

@utpilla utpilla Oct 3, 2022

Choose a reason for hiding this comment

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

EventSourceNames is also another good candidate to belong to EventCountersMetrics class. The options class won't be empty. It will still have the "options" 😀

Copy link
Contributor

Choose a reason for hiding this comment

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

In general, it falls more into the responsibility of the EventCountersMetrics class to determine what it should listen rather than an options class.

{
return this.eventSourceNames.Contains(eventSourceName);
}
}
Loading