Skip to content

Base extractor#522

Merged
einarmo merged 4 commits intomasterfrom
base-extractor
Mar 18, 2025
Merged

Base extractor#522
einarmo merged 4 commits intomasterfrom
base-extractor

Conversation

@einarmo
Copy link
Copy Markdown
Contributor

@einarmo einarmo commented Mar 3, 2025

First take on a new base extractor class. Some more work goes into this in the next step, adding startup reporting and integrating things a bit more tightly, but this is a nice first draft that contains some of the core logic.

The base extractor has a few roles, currently:

  • It contains the config object, in the future it will also handle changes to config revision and restart.
  • It acts as an error reporter, for raw errors.
  • It manages "monitored tasks", which are essentially just C# Tasks. The idea is that if something fails in a background process, this needs to be properly reported to the extractor so that it can take appropriate action. Background tasks have a name, can be cancelled independently, and can be expected to end or not, so if a task ends without an error, the extractor can take appropriate action.
  • It also contains shutdown logic, which is highly customizable, since extractor implementations may want to shut components down in a specific order. In general, different parts of the process are supposed to be independently cancelled, so that graceful shutdown is possible.

As with the last iteration of the base extractor, it is intended to be used as a base class for user extractors.

@einarmo einarmo requested a review from a team as a code owner March 3, 2025 06:31
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 3, 2025

Codecov Report

Attention: Patch coverage is 78.94737% with 44 lines in your changes missing coverage. Please review.

Project coverage is 78.24%. Comparing base (8b44bc4) to head (dc23272).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
ExtractorUtils/Unstable/BaseExtractor.cs 84.14% 9 Missing and 17 partials ⚠️
...ctorUtils/Unstable/Tasks/ExtractorTaskScheduler.cs 60.00% 11 Missing and 7 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #522      +/-   ##
==========================================
+ Coverage   78.10%   78.24%   +0.13%     
==========================================
  Files         123      124       +1     
  Lines       11541    11748     +207     
  Branches     1748     1781      +33     
==========================================
+ Hits         9014     9192     +178     
- Misses       1817     1822       +5     
- Partials      710      734      +24     
Files with missing lines Coverage Δ
ExtractorUtils/Unstable/Tasks/CheckInWorker.cs 76.29% <ø> (ø)
ExtractorUtils/Unstable/Tasks/Errors.cs 97.43% <ø> (ø)
...ctorUtils/Unstable/Tasks/ExtractorTaskScheduler.cs 65.04% <60.00%> (-1.12%) ⬇️
ExtractorUtils/Unstable/BaseExtractor.cs 84.14% <84.14%> (ø)

... and 4 files with indirect coverage changes

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Contributor

@ozangoktan ozangoktan left a comment

Choose a reason for hiding this comment

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

For the IIntegrationSink, I am starting to think sink may not be the best choice to describe it as. Isn't what it is doing something like an IOrchestratorIntegration rather than a sink adapting to an integration, which could mean other things than integrations api orchestrating the extractor.

Co-authored-by: Ozan Göktan <72358629+ozangoktan@users.noreply.github.com>
@einarmo
Copy link
Copy Markdown
Contributor Author

einarmo commented Mar 13, 2025

Let's do renaming the sink in a different PR, if we are doing it. It's a bit intrusive for this one.

@einarmo einarmo requested review from a team and finnag and removed request for a team March 13, 2025 11:32
Copy link
Copy Markdown

@finnag finnag left a comment

Choose a reason for hiding this comment

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

🦄

@einarmo einarmo merged commit 07f769e into master Mar 18, 2025
6 checks passed
@einarmo einarmo deleted the base-extractor branch March 18, 2025 09:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants