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.EntityFrameworkCore] Add Filter public API to enable filtering #1203

Merged
merged 8 commits into from
Jun 16, 2023

Conversation

akoken
Copy link
Contributor

@akoken akoken commented May 24, 2023

Changes

This PR introduces the addition of a Filter public API, which enables filtering out activities based on the properties of the
IDbCommand object being executed. The Filter API provides users with the ability to apply custom filters to only collect specific activities, enhancing the flexibility and versatility of the system.

For significant contributions please make sure you have completed the following items:

  • Appropriate CHANGELOG.md updated for non-trivial changes
  • Design discussion issue #
  • Changes in public API reviewed

@akoken akoken requested a review from a team May 24, 2023 14:25
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented May 24, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

@akoken akoken changed the title Add Filter public API to enable filtering [Instrumentation.EntityFrameworkCore] Add Filter public API to enable filtering May 24, 2023
@Kielek Kielek added the comp:instrumentation.entityframeworkcore Things related to OpenTelemetry.Instrumentation.EntityFrameworkCore label May 24, 2023
@Kielek
Copy link
Contributor

Kielek commented May 25, 2023

Did you considered usingEnrichWithIDbCommand? It should allow you to modify activity in context of IDbCommand.

@akoken
Copy link
Contributor Author

akoken commented May 25, 2023

Yes, we can modify activity with EnrichWithIDbCommand, but I don't think it's a right place to do that. Encrich and Filter should have their own purpose.

@Kielek
Copy link
Contributor

Kielek commented Jun 6, 2023

@akoken, I have checked it internally with other maintainers.

Please consider following Filter option Func<string, IDbCommand, bool> instead of Func<IDbCommand, bool>.

The string parameter could be potentially providerName.

@utpilla, @alanwest FYI

EDIT: markdown list is failing - probably to long lines.

@akoken
Copy link
Contributor Author

akoken commented Jun 6, 2023

@Kielek, I added providerName to the filter function and fixed the markdown issues.

@Kielek Kielek requested a review from utpilla June 9, 2023 07:27
@codecov
Copy link

codecov bot commented Jun 9, 2023

Codecov Report

Merging #1203 (1030309) into main (dbfeaea) will decrease coverage by 0.68%.
The diff coverage is 54.54%.

❗ Current head 1030309 differs from pull request most recent head c3d50ec. Consider uploading reports for the commit c3d50ec to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1203      +/-   ##
==========================================
- Coverage   73.61%   72.93%   -0.68%     
==========================================
  Files         265      265              
  Lines        9346     9301      -45     
==========================================
- Hits         6880     6784      -96     
- Misses       2466     2517      +51     
Impacted Files Coverage Δ
...ry.Exporter.Geneva/Metrics/GenevaMetricExporter.cs 70.03% <ø> (-4.52%) ⬇️
...ation/EntityFrameworkInstrumentationEventSource.cs 12.00% <28.57%> (+12.00%) ⬆️
...mplementation/EntityFrameworkDiagnosticListener.cs 52.38% <64.28%> (+1.48%) ⬆️
...eworkCore/EntityFrameworkInstrumentationOptions.cs 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

akoken and others added 2 commits June 16, 2023 20:49
@Kielek Kielek merged commit 68e60db into open-telemetry:main Jun 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:instrumentation.entityframeworkcore Things related to OpenTelemetry.Instrumentation.EntityFrameworkCore
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants