Add basic Scala Weaver sbt support #8189
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![datadog-datadog-prod-us1[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
Comment on lines
15
to
19
| public TaskDefAwareQueueProxy(TaskDef taskDef, ConcurrentLinkedQueue<T> delegate) { | ||
| this.taskDef = taskDef; | ||
| this.delegate = delegate; | ||
| DatadogWeaverReporter.start(); | ||
| } |
There was a problem hiding this comment.
⚪ Code Quality Violation
Consider adding super() or this() to your constructor (...read more)
In Java, it is suggested to call super() in an extended class. This rule will report a violation if both a call to super() and an overloaded constructor are absent.
daniel-mohedano
added
type: enhancement
comp: ci visibility
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 1 performance regressions! Performance is the same for 58 metrics, 4 unstable metrics.
Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.46.0-SNAPSHOT~af569223da, baseline=1.46.0-SNAPSHOT~296ed7bdd3
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.064 s) : 0, 1063816
Total [baseline] (8.635 s) : 0, 8634646
Agent [candidate] (1.06 s) : 0, 1059693
Total [candidate] (8.63 s) : 0, 8630041
section iast
Agent [baseline] (1.181 s) : 0, 1181000
Total [baseline] (9.174 s) : 0, 9173789
Agent [candidate] (1.186 s) : 0, 1186164
Total [candidate] (9.255 s) : 0, 9254817
section iast_HARDCODED_SECRET_DISABLED
Agent [baseline] (1.193 s) : 0, 1192734
Total [baseline] (9.214 s) : 0, 9213871
Agent [candidate] (1.183 s) : 0, 1182881
Total [candidate] (9.175 s) : 0, 9174564
section iast_TELEMETRY_OFF
Agent [baseline] (1.184 s) : 0, 1183917
Total [baseline] (9.239 s) : 0, 9239058
Agent [candidate] (1.18 s) : 0, 1179967
Total [candidate] (9.216 s) : 0, 9215545
gantt
title insecure-bank - break down per module: candidate=1.46.0-SNAPSHOT~af569223da, baseline=1.46.0-SNAPSHOT~296ed7bdd3
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (719.641 ms) : 0, 719641
BytebuddyAgent [candidate] (715.955 ms) : 0, 715955
GlobalTracer [baseline] (258.092 ms) : 0, 258092
GlobalTracer [candidate] (256.675 ms) : 0, 256675
AppSec [baseline] (56.441 ms) : 0, 56441
AppSec [candidate] (55.428 ms) : 0, 55428
Remote Config [baseline] (725.462 µs) : 0, 725
Remote Config [candidate] (716.629 µs) : 0, 717
Telemetry [baseline] (13.748 ms) : 0, 13748
Telemetry [candidate] (15.962 ms) : 0, 15962
section iast
BytebuddyAgent [baseline] (830.301 ms) : 0, 830301
BytebuddyAgent [candidate] (834.033 ms) : 0, 834033
GlobalTracer [baseline] (246.689 ms) : 0, 246689
GlobalTracer [candidate] (247.61 ms) : 0, 247610
AppSec [baseline] (58.066 ms) : 0, 58066
AppSec [candidate] (58.295 ms) : 0, 58295
Remote Config [baseline] (661.004 µs) : 0, 661
Remote Config [candidate] (684.433 µs) : 0, 684
Telemetry [baseline] (8.805 ms) : 0, 8805
Telemetry [candidate] (8.817 ms) : 0, 8817
IAST [baseline] (21.419 ms) : 0, 21419
IAST [candidate] (21.612 ms) : 0, 21612
section iast_HARDCODED_SECRET_DISABLED
BytebuddyAgent [baseline] (838.93 ms) : 0, 838930
BytebuddyAgent [candidate] (831.428 ms) : 0, 831428
GlobalTracer [baseline] (248.296 ms) : 0, 248296
GlobalTracer [candidate] (247.148 ms) : 0, 247148
AppSec [baseline] (58.484 ms) : 0, 58484
AppSec [candidate] (58.331 ms) : 0, 58331
Remote Config [baseline] (707.454 µs) : 0, 707
Remote Config [candidate] (686.223 µs) : 0, 686
Telemetry [baseline] (9.047 ms) : 0, 9047
Telemetry [candidate] (8.829 ms) : 0, 8829
IAST [baseline] (21.975 ms) : 0, 21975
IAST [candidate] (21.447 ms) : 0, 21447
section iast_TELEMETRY_OFF
BytebuddyAgent [baseline] (832.228 ms) : 0, 832228
BytebuddyAgent [candidate] (829.028 ms) : 0, 829028
GlobalTracer [baseline] (247.965 ms) : 0, 247965
GlobalTracer [candidate] (247.649 ms) : 0, 247649
AppSec [baseline] (58.223 ms) : 0, 58223
AppSec [candidate] (58.119 ms) : 0, 58119
Remote Config [baseline] (669.806 µs) : 0, 670
Remote Config [candidate] (671.021 µs) : 0, 671
Telemetry [baseline] (8.717 ms) : 0, 8717
Telemetry [candidate] (8.652 ms) : 0, 8652
IAST [baseline] (21.077 ms) : 0, 21077
IAST [candidate] (20.807 ms) : 0, 20807
Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.46.0-SNAPSHOT~af569223da, baseline=1.46.0-SNAPSHOT~296ed7bdd3
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.061 s) : 0, 1061452
Total [baseline] (10.463 s) : 0, 10462901
Agent [candidate] (1.058 s) : 0, 1058477
Total [candidate] (10.388 s) : 0, 10388276
section appsec
Agent [baseline] (1.186 s) : 0, 1185720
Total [baseline] (10.691 s) : 0, 10691326
Agent [candidate] (1.192 s) : 0, 1192326
Total [candidate] (10.71 s) : 0, 10710449
section iast
Agent [baseline] (1.183 s) : 0, 1182990
Total [baseline] (10.937 s) : 0, 10936966
Agent [candidate] (1.191 s) : 0, 1190834
Total [candidate] (10.947 s) : 0, 10947426
section profiling
Agent [baseline] (1.268 s) : 0, 1267698
Total [baseline] (10.881 s) : 0, 10881409
Agent [candidate] (1.272 s) : 0, 1272500
Total [candidate] (10.825 s) : 0, 10824518
gantt
title petclinic - break down per module: candidate=1.46.0-SNAPSHOT~af569223da, baseline=1.46.0-SNAPSHOT~296ed7bdd3
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (718.888 ms) : 0, 718888
BytebuddyAgent [candidate] (715.798 ms) : 0, 715798
GlobalTracer [baseline] (257.381 ms) : 0, 257381
GlobalTracer [candidate] (256.863 ms) : 0, 256863
AppSec [baseline] (56.352 ms) : 0, 56352
AppSec [candidate] (56.949 ms) : 0, 56949
Remote Config [baseline] (736.006 µs) : 0, 736
Remote Config [candidate] (736.943 µs) : 0, 737
Telemetry [baseline] (12.988 ms) : 0, 12988
Telemetry [candidate] (13.122 ms) : 0, 13122
section appsec
BytebuddyAgent [baseline] (729.578 ms) : 0, 729578
BytebuddyAgent [candidate] (734.179 ms) : 0, 734179
GlobalTracer [baseline] (252.377 ms) : 0, 252377
GlobalTracer [candidate] (253.276 ms) : 0, 253276
AppSec [baseline] (170.349 ms) : 0, 170349
AppSec [candidate] (170.66 ms) : 0, 170660
Remote Config [baseline] (656.452 µs) : 0, 656
Remote Config [candidate] (667.084 µs) : 0, 667
Telemetry [baseline] (8.127 ms) : 0, 8127
Telemetry [candidate] (8.635 ms) : 0, 8635
IAST [baseline] (19.35 ms) : 0, 19350
IAST [candidate] (19.528 ms) : 0, 19528
section iast
BytebuddyAgent [baseline] (831.929 ms) : 0, 831929
BytebuddyAgent [candidate] (837.355 ms) : 0, 837355
GlobalTracer [baseline] (246.985 ms) : 0, 246985
GlobalTracer [candidate] (248.693 ms) : 0, 248693
AppSec [baseline] (57.99 ms) : 0, 57990
AppSec [candidate] (58.199 ms) : 0, 58199
Remote Config [baseline] (699.164 µs) : 0, 699
Remote Config [candidate] (685.618 µs) : 0, 686
Telemetry [baseline] (8.819 ms) : 0, 8819
Telemetry [candidate] (8.957 ms) : 0, 8957
IAST [baseline] (21.542 ms) : 0, 21542
IAST [candidate] (21.802 ms) : 0, 21802
section profiling
BytebuddyAgent [baseline] (708.631 ms) : 0, 708631
BytebuddyAgent [candidate] (702.543 ms) : 0, 702543
GlobalTracer [baseline] (354.259 ms) : 0, 354259
GlobalTracer [candidate] (369.886 ms) : 0, 369886
AppSec [baseline] (55.583 ms) : 0, 55583
AppSec [candidate] (53.927 ms) : 0, 53927
Remote Config [baseline] (686.591 µs) : 0, 687
Remote Config [candidate] (693.742 µs) : 0, 694
Telemetry [baseline] (9.088 ms) : 0, 9088
Telemetry [candidate] (8.862 ms) : 0, 8862
ProfilingAgent [baseline] (97.437 ms) : 0, 97437
ProfilingAgent [candidate] (94.686 ms) : 0, 94686
Profiling [baseline] (97.461 ms) : 0, 97461
Profiling [candidate] (94.713 ms) : 0, 94713
LoadParameters
See matching parameters
SummaryFound 1 performance improvements and 1 performance regressions! Performance is the same for 10 metrics, 16 unstable metrics.
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.46.0-SNAPSHOT~af569223da, baseline=1.46.0-SNAPSHOT~296ed7bdd3
dateFormat X
axisFormat %s
section baseline
no_agent (1.357 ms) : 1337, 1378
. : milestone, 1357,
appsec (1.724 ms) : 1700, 1748
. : milestone, 1724,
appsec_no_iast (1.766 ms) : 1741, 1792
. : milestone, 1766,
iast (1.5 ms) : 1476, 1525
. : milestone, 1500,
profiling (1.515 ms) : 1491, 1540
. : milestone, 1515,
tracing (1.485 ms) : 1460, 1511
. : milestone, 1485,
section candidate
no_agent (1.354 ms) : 1333, 1374
. : milestone, 1354,
appsec (1.786 ms) : 1763, 1809
. : milestone, 1786,
appsec_no_iast (1.763 ms) : 1739, 1786
. : milestone, 1763,
iast (1.486 ms) : 1463, 1509
. : milestone, 1486,
profiling (1.517 ms) : 1494, 1540
. : milestone, 1517,
tracing (1.504 ms) : 1479, 1529
. : milestone, 1504,
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.46.0-SNAPSHOT~af569223da, baseline=1.46.0-SNAPSHOT~296ed7bdd3
dateFormat X
axisFormat %s
section baseline
no_agent (381.489 µs) : 361, 402
. : milestone, 381,
iast (502.682 µs) : 481, 524
. : milestone, 503,
iast_FULL (742.806 µs) : 721, 765
. : milestone, 743,
iast_GLOBAL (552.923 µs) : 531, 575
. : milestone, 553,
iast_HARDCODED_SECRET_DISABLED (515.335 µs) : 493, 537
. : milestone, 515,
iast_INACTIVE (463.077 µs) : 441, 485
. : milestone, 463,
iast_TELEMETRY_OFF (498.094 µs) : 476, 520
. : milestone, 498,
tracing (455.8 µs) : 435, 477
. : milestone, 456,
section candidate
no_agent (381.764 µs) : 362, 402
. : milestone, 382,
iast (499.918 µs) : 478, 522
. : milestone, 500,
iast_FULL (661.365 µs) : 640, 683
. : milestone, 661,
iast_GLOBAL (528.667 µs) : 506, 551
. : milestone, 529,
iast_HARDCODED_SECRET_DISABLED (501.675 µs) : 480, 523
. : milestone, 502,
iast_INACTIVE (461.41 µs) : 440, 483
. : milestone, 461,
iast_TELEMETRY_OFF (493.33 µs) : 471, 516
. : milestone, 493,
tracing (454.968 µs) : 434, 476
. : milestone, 455,
DacapoParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 12 metrics, 0 unstable metrics. Execution time for tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.46.0-SNAPSHOT~af569223da, baseline=1.46.0-SNAPSHOT~296ed7bdd3
dateFormat X
axisFormat %s
section baseline
no_agent (1.475 ms) : 1463, 1487
. : milestone, 1475,
appsec (2.365 ms) : 2322, 2407
. : milestone, 2365,
iast (2.117 ms) : 2063, 2171
. : milestone, 2117,
iast_GLOBAL (2.168 ms) : 2113, 2223
. : milestone, 2168,
profiling (1.976 ms) : 1933, 2020
. : milestone, 1976,
tracing (1.946 ms) : 1904, 1988
. : milestone, 1946,
section candidate
no_agent (1.477 ms) : 1466, 1489
. : milestone, 1477,
appsec (2.373 ms) : 2330, 2416
. : milestone, 2373,
iast (2.114 ms) : 2060, 2169
. : milestone, 2114,
iast_GLOBAL (2.161 ms) : 2107, 2216
. : milestone, 2161,
profiling (1.989 ms) : 1945, 2033
. : milestone, 1989,
tracing (1.951 ms) : 1909, 1993
. : milestone, 1951,
Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.46.0-SNAPSHOT~af569223da, baseline=1.46.0-SNAPSHOT~296ed7bdd3
dateFormat X
axisFormat %s
section baseline
no_agent (15.455 s) : 15455000, 15455000
. : milestone, 15455000,
appsec (15.122 s) : 15122000, 15122000
. : milestone, 15122000,
iast (18.715 s) : 18715000, 18715000
. : milestone, 18715000,
iast_GLOBAL (17.996 s) : 17996000, 17996000
. : milestone, 17996000,
profiling (15.249 s) : 15249000, 15249000
. : milestone, 15249000,
tracing (15.03 s) : 15030000, 15030000
. : milestone, 15030000,
section candidate
no_agent (14.931 s) : 14931000, 14931000
. : milestone, 14931000,
appsec (15.013 s) : 15013000, 15013000
. : milestone, 15013000,
iast (18.678 s) : 18678000, 18678000
. : milestone, 18678000,
iast_GLOBAL (17.872 s) : 17872000, 17872000
. : milestone, 17872000,
profiling (15.003 s) : 15003000, 15003000
. : milestone, 15003000,
tracing (14.965 s) : 14965000, 14965000
. : milestone, 14965000,
|
| @Nullable String testMethodName, | ||
| @Nullable Method testMethod, | ||
| boolean isRetry, | ||
| @Nullable Long startTime); |
There was a problem hiding this comment.
Let's just add this startTime parameter to the existing method to keep the interface simple. You can use Refactor -> Change Signature (cmd+F6) in Intellij, it will do all the work for you (you can specify the default value of null for the new parameter, and the IDE will update all the callers of the method).
There was a problem hiding this comment.
Refactored also TestSuite start and end handling to accept time parameters, to keep the interface consistent.
| } | ||
|
|
||
| @Override | ||
| public boolean isApplicable(Set<TargetSystem> enabledSystems) { |
There was a problem hiding this comment.
There's no need to override this method, since all it does is delegating to the parent class
| @@ -504,6 +504,7 @@ include ':dd-java-agent:instrumentation:redisson' | |||
| include ':dd-java-agent:instrumentation:redisson:redisson-2.0.0' | |||
| include ':dd-java-agent:instrumentation:redisson:redisson-2.3.0' | |||
| include ':dd-java-agent:instrumentation:redisson:redisson-3.10.3' | |||
| include ':dd-java-agent:instrumentation:weaver' | |||
There was a problem hiding this comment.
We should also update isTestingInstrumentation in gradle/configure_tests.gradle
|
Hi! Original feature request author here. I'm excited to see this being worked on, and as far as I can tell, the changes look great! I would suggest adding a few extra tests beyond the SimpleIOSuite. Specifically, testing the IOSuite with suite resource sharing and IOSuite using global resource would be beneficial. Examples from the documentation should be sufficient for these test cases. If you need more details on Weaver, I’m happy to help! |
|
Thanks @majk-p ! I'll take into account. In order for us to prioritize future work once these changes are merged, are there any specific features you'd be interested in (with regards to CIVis/TestOptimization)? |
|
I need to double check with my team on the requirements. For context it's worth noting that my team specifically builds a framework on top of weaver. For sure 3 things come to my mind:
To overcome the missing weaver support we implemented CI Visibility by following this guide https://docs.datadoghq.com/tests/setup/java/?tab=ciproviderwithautoinstrumentationsupport#using-manual-testing-api. if (!testName.tags.contains(TestName.Tags.ignore))
error match {
case ignored: weaver.IgnoredException => ignored.reason.foreach(test.setSkipReason(_))
case other => test.setErrorInfo(other)
}
When using test visibility api directly we were spawning objects like final Span span = GlobalTracer.get().activeSpan();
if (span != null) {
span.setTag("test_owner", "my_team");
}For context, weaver uses Cats Effect so the tests are executed in parallel on something like green threads. Can you confirm this approach is thread safe?
One of the limitations of my current approach is that I cannot link the test visibility Tracing tab to the http requests done during the test. What I'd like to be able to do is to somehow obtain the trace id of the test being executed and pass it in http headers to see them rendered in tracing tab of test execution on the web panel. |
|
Thanks for letting us know! Point 1 should already be handled by the changes in this PR, so both ignored and canceled tests should be correctly marked as skipped in DataDog. Points 2 and 3 are a bit more tricky, and won't work for now with the current changes. But we will focus on them next in order to support the functionality. |
|
Great, I'm looking forward for all those changes! If there's a snapshot version available I'm happy to give it a try and share my feedback |
|
Of course! You can access a snapshot version here. Let me know if you have any issues. |
|
I was trying hard to make to try it out, but I must be doing something wrong because I see no test runs with Here's what I did:
I made sure that
Am I misconfiguring the env? Here are the logs printed by the agent: |
|
No problem! Try with |
|
Thanks for the tip, you were right. It worked locally! To test this on CI, do you think there's a way to tell the agent script that we install via https://install.datadoghq.com/scripts/install_script_agent7.sh to use this specific build? |
|
On a related note, when weaver is officially supported it will probably get listed on https://docs.datadoghq.com/tests/setup/java. Do you the https://docs.datadoghq.com/tests/setup/java/?tab=gradle#running-your-tests section can be extended with |
|
That's great news! Unfortunately, the single-step instrumentation script doesn't allow to install the tracer from an arbitrary URL. To test the changes in CI right now you would have to update your pipeline to download the tracer and configure the env vars as indicated in the docs. With that said, the changes should be available with the next release of the tracer, scheduled for the first week of February (or even sooner than that if there is a minor release). The documentation will be updated too to reflect the changes. I still have to discuss with the team how we want to approach adding |
|
Thanks Daniel! I managed to work around the script setting things up manually and confirmed the instrumentation works on CI as well. Now that I know it works in both envs, I'm happy to wait for the release. Great job! As for documentation that's totally understandable. I'm happy to share feedback and examples of what was working for us for sbt & Scala setup. |
|
Glad to hear that! And thanks for the feedback. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What Does This Do
Adds support for Scala Weaver testing framework (using sbt) to CI Visibility.
Motivation
There is a feature request.
Additional Notes
Contributor Checklist
type:and (comp:orinst:) labels in addition to any usefull labelsclose,fixor any linking keywords when referencing an issue.Use
solvesinstead, and assign the PR milestone to the issueJira ticket: SDTEST-1439