Create an EventCounter Listener that subscribes to all event counters #365

hananiel · 2022-05-18T15:07:25Z

Resolves Issue #215

Created a focussed PR that subscribes to all Event Counter Events.
The configuration to selectively listen to particular EventCounter Sources and/or Counters will be added as a separate PR.

This will replace PR #177

codecov · 2022-05-18T16:13:56Z

Codecov Report

Merging #365 (362558a) into main (f27d824) will not change coverage.
The diff coverage is 0.00%.

@@          Coverage Diff          @@
##            main    #365   +/-   ##
=====================================
  Coverage   0.00%   0.00%           
=====================================
  Files        163     167    +4     
  Lines       4996    5084   +88     
=====================================
- Misses      4996    5084   +88

Impacted Files	Coverage Δ
...trumentation.EventCounters/EventCounterListener.cs	`0.00% <0.00%> (ø)`
...tation.EventCounters/EventCounterMetricsOptions.cs	`0.00% <0.00%> (ø)`
...ounters/EventCountersInstrumentationEventSource.cs	`0.00% <0.00%> (ø)`
...on.EventCounters/MeterProviderBuilderExtensions.cs	`0.00% <0.00%> (ø)`

src/OpenTelemetry.Instrumentation.EventCounters/EventCounterListener.cs

cijothomas · 2022-05-27T20:14:55Z

src/OpenTelemetry.Instrumentation.EventCounters/EventCounterListener.cs

+
+        protected override void OnEventWritten(EventWrittenEventArgs eventData)
+        {
+            if (!this.isInitialized || !eventData.EventName.Equals("EventCounters"))


Where does the "EventCounters" name come from?

It is hardhoded in the implementation class: https://github.com/dotnet/runtime/blob/e026f392b2bb93bc9509ac5403d3e8cd7d3eee72/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventCounter.cs#L129

nit: I think it might be better to swap the order of the conditions here.

src/OpenTelemetry.Instrumentation.EventCounters/EventCounterListener.cs

birojnayak · 2022-06-06T03:20:10Z

Resolves Issue #215

Created a focussed PR that subscribes to all Event Counter Events. The configuration to selectively listen to particular EventCounter Sources and/or Counters will be added as a separate PR.

This will replace PR #177

this is definitely going to be useful.. waiting for this CR to be merged

github-actions · 2022-06-18T03:33:09Z

This PR was marked stale due to lack of activity. It will be closed in 7 days.

stebet · 2022-06-23T10:21:18Z

Would absolutely love to see this make it in! Let me know if I can help @cijothomas @hananiel

github-actions · 2022-07-01T03:48:39Z

This PR was marked stale due to lack of activity. It will be closed in 7 days.

cijothomas

Looks good for an initial PR. We can iterate on this.

utpilla · 2022-07-08T18:58:25Z

src/OpenTelemetry.Instrumentation.EventCounters/README.md

+### Step 1: Install Package
+
+Add a reference to the
+[`OpenTelemetry.Contrib.Instrumentation.EventCounters`](https://www.nuget.org/packages/OpenTelemetry.Contrib.Instrumentation.EventCounters)


Remove the word Contrib.

src/OpenTelemetry.Instrumentation.EventCounters/README.md

utpilla · 2022-07-08T19:00:04Z

...enTelemetry.Instrumentation.EventCounters/OpenTelemetry.Instrumentation.EventCounters.csproj

+        <Nullable>enable</Nullable>
+    </PropertyGroup>
+    <ItemGroup>
+        <PackageReference Include="OpenTelemetry.Api" Version="1.2.0-rc2" />


You could use 1.3.0.

utpilla · 2022-07-08T19:15:09Z

src/OpenTelemetry.Instrumentation.EventCounters/EventCounterListener.cs

+                case InstrumentType.ObservableCounter:
+                    if (!this.metricInstruments.ContainsKey(metricKey))
+                    {
+                        this.metricInstruments[metricKey] = this.meter.CreateObservableCounter(counterName, () => this.ObserveDouble(metricKey), description: description);


Use the TryAdd method of the ConcurrentDictionary for a thread-safe implementation.

utpilla · 2022-07-08T19:21:24Z

@hananiel Thanks for creating this PR! Could you please follow the contributing doc to add additional files such as package workflow, issue template etc. You could also refer to any of the existing projects.

The other comments could be addressed in a follow-up PR.

utpilla

Approved with suggestions.

src/OpenTelemetry.Instrumentation.EventCounters/EventCounterListener.cs

utpilla · 2022-07-12T21:34:00Z

test/OpenTelemetry.Instrumentation.EventCounters.Tests/EventCounterListenerTests.cs

+            Assert.True(metricItems.Count > 1);
+        }
+
+        [Fact(Skip = "Unstable")]


Consider using InMemoryExporter that takes in a collection of MetricSnapshot as we persist the metric values in that case.

utpilla · 2022-07-12T21:36:51Z

test/OpenTelemetry.Instrumentation.EventCounters.Tests/EventCounterListenerTests.cs

+        }
+
+        [Fact(Skip = "Unstable")]
+        public async Task TestEventCounterMetricsAreCaptured()


These need not be async Task. You could use Task.Delay(...).Wait() instead.

utpilla

LGTM. Left some suggestions to improve the unit tests which could be done in a follow-up PR.

magoogli · 2022-08-01T18:14:17Z

This feature is great @hananiel, just what I was looking for. Can I help with the additional work to select which event counters to listen to, or is this in progress already?

mic-max · 2022-09-06T22:31:08Z

This feature is great @hananiel, just what I was looking for. Can I help with the additional work to select which event counters to listen to, or is this in progress already?

I did some work on that here #620
There could be additional need to listen to an eventName from a specific eventSource though. The options I configured only listen to the listed event sources and listed event names, not a particular event from a specific event source if that makes sense.

hananiel requested a review from a team May 18, 2022 15:07

Create a EventCounter Listener that subscribes to all event counters

c87eef6

hananiel force-pushed the EventCounterListenerWithoutConfig branch from aac78af to c87eef6 Compare May 18, 2022 16:05

hananiel changed the title ~~Create a EventCounter Listener that subscribes to all event counters~~ Create an EventCounter Listener that subscribes to all event counters May 18, 2022

Fix lint and format errors

7debc15

hananiel force-pushed the EventCounterListenerWithoutConfig branch from ea5cf17 to 7debc15 Compare May 18, 2022 16:57

Merge branch 'main' into EventCounterListenerWithoutConfig

2cbde61

cijothomas reviewed May 26, 2022

View reviewed changes

src/OpenTelemetry.Instrumentation.EventCounters/EventCounterListener.cs Outdated Show resolved Hide resolved

cijothomas reviewed May 26, 2022

View reviewed changes

src/OpenTelemetry.Instrumentation.EventCounters/EventCounterListener.cs Outdated Show resolved Hide resolved

cijothomas reviewed May 26, 2022

View reviewed changes

src/OpenTelemetry.Instrumentation.EventCounters/EventCounterListener.cs Outdated Show resolved Hide resolved

cijothomas reviewed May 26, 2022

View reviewed changes

src/OpenTelemetry.Instrumentation.EventCounters/EventCounterListener.cs Show resolved Hide resolved

CR feedback

290d309

hananiel requested a review from cijothomas May 26, 2022 22:35

cijothomas reviewed May 27, 2022

View reviewed changes

cijothomas reviewed May 28, 2022

View reviewed changes

src/OpenTelemetry.Instrumentation.EventCounters/EventCounterListener.cs Outdated Show resolved Hide resolved

cijothomas reviewed May 28, 2022

View reviewed changes

src/OpenTelemetry.Instrumentation.EventCounters/EventCounterListener.cs Show resolved Hide resolved

hananiel mentioned this pull request May 31, 2022

Event counter listener #177

Closed

CR feedback

ef1b937

hananiel requested a review from cijothomas June 10, 2022 19:50

github-actions bot added the Stale label Jun 18, 2022

github-actions bot removed the Stale label Jun 24, 2022

github-actions bot added the Stale label Jul 1, 2022

cijothomas removed the Stale label Jul 7, 2022

Resolve merge conflicts

92e9433

cijothomas approved these changes Jul 8, 2022

View reviewed changes

hananiel added 3 commits July 8, 2022 11:42

Fix file encoding

71a56ec

fix line endings

d8bac49

Fix timing issues on build server; Add net6.0

5c7bff8

utpilla reviewed Jul 8, 2022

View reviewed changes

src/OpenTelemetry.Instrumentation.EventCounters/README.md Show resolved Hide resolved

utpilla reviewed Jul 8, 2022

View reviewed changes

utpilla approved these changes Jul 8, 2022

View reviewed changes

cijothomas and others added 2 commits July 11, 2022 13:18

Merge branch 'main' into EventCounterListenerWithoutConfig

002c659

Add workflow, owners, issueTemplate, resolve CR feedback

89004e1

github-actions bot assigned hananiel Jul 12, 2022

hananiel and others added 3 commits July 12, 2022 14:01

Fix MD lint

c5d1b5a

Add Changelog

6bfe5c5

Merge branch 'main' into EventCounterListenerWithoutConfig

e36fa65

utpilla reviewed Jul 12, 2022

View reviewed changes

src/OpenTelemetry.Instrumentation.EventCounters/EventCounterListener.cs Outdated Show resolved Hide resolved

utpilla reviewed Jul 12, 2022

View reviewed changes

src/OpenTelemetry.Instrumentation.EventCounters/EventCounterListener.cs Outdated Show resolved Hide resolved

hananiel added 4 commits July 12, 2022 15:02

Add release date to changelog

d44e6ea

Fix markdown

3788097

Skip unstable tests

9d18b7b

Remove check for dictionary on tryAdd

362558a

utpilla reviewed Jul 12, 2022

View reviewed changes

utpilla approved these changes Jul 12, 2022

View reviewed changes

utpilla merged commit 9418c08 into open-telemetry:main Jul 12, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create an EventCounter Listener that subscribes to all event counters #365

Create an EventCounter Listener that subscribes to all event counters #365

hananiel commented May 18, 2022

codecov bot commented May 18, 2022 •

edited

Loading

cijothomas May 27, 2022

hananiel Jun 10, 2022

utpilla Jul 8, 2022

birojnayak commented Jun 6, 2022

github-actions bot commented Jun 18, 2022

stebet commented Jun 23, 2022

github-actions bot commented Jul 1, 2022

cijothomas left a comment

utpilla Jul 8, 2022

utpilla Jul 8, 2022

utpilla Jul 8, 2022 •

edited

Loading

utpilla commented Jul 8, 2022

utpilla left a comment

utpilla Jul 12, 2022

utpilla Jul 12, 2022

utpilla left a comment

magoogli commented Aug 1, 2022

mic-max commented Sep 6, 2022

Create an EventCounter Listener that subscribes to all event counters #365

Create an EventCounter Listener that subscribes to all event counters #365

Conversation

hananiel commented May 18, 2022

codecov bot commented May 18, 2022 • edited Loading

Codecov Report

cijothomas May 27, 2022

Choose a reason for hiding this comment

hananiel Jun 10, 2022

Choose a reason for hiding this comment

utpilla Jul 8, 2022

Choose a reason for hiding this comment

birojnayak commented Jun 6, 2022

github-actions bot commented Jun 18, 2022

stebet commented Jun 23, 2022

github-actions bot commented Jul 1, 2022

cijothomas left a comment

Choose a reason for hiding this comment

utpilla Jul 8, 2022

Choose a reason for hiding this comment

utpilla Jul 8, 2022

Choose a reason for hiding this comment

utpilla Jul 8, 2022 • edited Loading

Choose a reason for hiding this comment

utpilla commented Jul 8, 2022

utpilla left a comment

Choose a reason for hiding this comment

utpilla Jul 12, 2022

Choose a reason for hiding this comment

utpilla Jul 12, 2022

Choose a reason for hiding this comment

utpilla left a comment

Choose a reason for hiding this comment

magoogli commented Aug 1, 2022

mic-max commented Sep 6, 2022

codecov bot commented May 18, 2022 •

edited

Loading

utpilla Jul 8, 2022 •

edited

Loading