-
Notifications
You must be signed in to change notification settings - Fork 447
chore: refactor SpanAggregator <> TraceWriter interface #13894
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
base: main
Are you sure you want to change the base?
Conversation
Help remove some implementation details from SpanAggregator into a the writer module.
This is going to have conflicts with #13876 |
|
@@ -80,8 +84,7 @@ def _human_size(nbytes): | |||
|
|||
class TraceWriter(metaclass=abc.ABCMeta): | |||
@abc.abstractmethod | |||
def recreate(self): | |||
# type: () -> TraceWriter | |||
def recreate(self, appsec_enabled: Optional[bool] = None) -> "TraceWriter": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't find a good way around this, since the value is coming from tracer.configure
and not tied to a specific config option... we probably need to keep this until we can deprecate appsec_enabled
from tracer.configure()
:/
Bootstrap import analysisComparison of import times between this PR and base. SummaryThe average import time from this PR is: 282 ± 4 ms. The average import time from base is: 286 ± 4 ms. The import time difference between this PR and base is: -3.8 ± 0.2 ms. Import time breakdownThe following import paths have appeared:
|
BenchmarksBenchmark execution time: 2025-07-10 16:01:41 Comparing candidate commit 8f58bf4 in PR branch Found 0 performance improvements and 2 performance regressions! Performance is the same for 480 metrics, 2 unstable metrics. scenario:iastaspectsospath-ospathbasename_aspect
scenario:iastaspectsospath-ospathsplitext_aspect
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, this will also be helpful for the DummyWriter creation with NativeWriter
Help remove some implementation details from SpanAggregator into a the writer module.
Checklist
Reviewer Checklist