Enabling the spring-data integration in this test
Mostly, this introduces an extra span for the repository; however, it also connects some previously separate traces under the repository action.
To shift the set-up timing without adding a lot of ugly code (as I did previously), I introduced a lazy dynamic proxy around the repository.
I'm not really happy with this, but I consider it better than the prior approach. There is also probably a more "groovy" way to do this.
As before, this change is itself non-functional. The subsequent test will enable spring-data instrumentation and alter the test.
@Shared runs before the spring-data instrumentation is enabled.
Switching to more explicitly set-up code to change the timing; however, this change doesn't enabled spring-data instrumentation to keep this non-functional.
After seeing the existing elasticsearch 5.3 spring data test, I decided to extend that instead.
However, I do think we need to revisit the best place for tests involving multiple pieces of instrumentation.
Switched to extending JpaRepository rather than CrudRepository, so that multiple spring data integrations can coexist.
Since JpaRepository redefines findAll, this required one update to the test. The other references to CrudRepositopry remain unchanged since JpaRepository extends CrudRepository without redefining those methods.
Rather than using a somewhat ugly solution of implementing both postProcess method signatures.
I'm restricting the integration to newer versions (1.9 - Sept 2014) of spring-data to start.
RepositoryProxyPostProcessor has different method sigs depending on the version of spring-data-commons.
As stop gap implemented both signatures, but probably need to split spring-data support by version.
This revised version targets spring-data RepositoryFactorySupport rather than spring-tx
This is accomplished by injecting a RepositoryProxyPostProcessor during construction that adds a datadog specific MethodIntercptor.
In the end, this functions similarly to the tx support in that it uses a widely scoped MethodInterceptor.
This required some changes to the test as well. The old test set-up the repository before the instrumentation was fully-enabled. Enabling the instrumentation earlier capture extra traces where Spring JPA performs metainfo queries.
This allows trace propagation for CompletableFuture’s asyncPool even if `useCommonPool` is disabled.
Also added some additional futures to `WHITELISTED_FUTURES` and sorted list.
The SpringDataDecorator is really for Spring transactions including non-DB related transactions.
The SpringDataDecorator never really implemented DB or ORM support anyway, since most of the methods just returned null.
Changing the decorator to just extend ClientDecorator instead. Also rename to reflect its true purpose.
Switching muzzle to target spring-tx rather than spring-data (which is logical given the instrumentation point)
Also added extraDependency on aopalliance
This restricts the support the Spring 2.5 when spring-tx & spring-aop using aopalliance were introduced.
After experimenting with RepositorySupport$QueryExecutorMethodInterceptor switched to TransactionInterceptor.
QueryExecutorMethodInterceptor was too narrowly scoped and didn't capture the UPDATE or DELETE statement. TransactionInterceptor captures everything, but is still a bit more broadly applicable than I'd like.
Unfortunately 404 test was not consistent and non-trivial to adapt. I also couldn’t get the validation tests to work well either. Revisit if we have time to dig deeper into spring.
Added some explanatory comments for each span -- might turn these into assertions later
Primary aim was to understand the differences from the signalfx fork. They seem to stem deviations in the underlying JDBC integration.
This is a more generic form of the previous `split-by-domain` and `split-by-instance`.
(Note: evaluation is done when a tag is set, so If multiple tags are configured, the last tag set will take precidence.)
For example, this setting can be used to rename `aws-sdk` spans to be identified with the corresponding `aws.service` tag:
```
-Ddd.trace.split-by-tags=aws.service
```
There are some slight differences with this setting compared with`dd.trace.http.client.split-by-domain` and `dd.trace.db.client.split-by-instance`. `split-by-tags` applies to every span, where `split-by-domain` and `split-by-instance` only apply to http and db client spans respectively.
The core changes are in Config and ServerDecorator.
Moved default tagging from Config::getRuntimeTags to Config::getLocalRootSpanTags. This changes the result of Config::getMergedJmxTags as well.
To preserve language for servers changed ServerDecorator::afterStart.
Other changes are in tests - the most complicated part is in TagsAssert::defaultTags. This now contains a bit too much conditional logic for my liking.
Otherwise it can interfere with the more common Servlet instrumentation (changing the root span name).
Unify attribute/property name for saving span on a request/context.
Also add tests for embedded GlassFish.
(Don’t be surprised when things break with 2.8… They’re religious about removing deprecated methods on minor release versions. If they followed standard convention, they’d likely be on at least 11.x.)
Add client test for 2.4-2.5 http library. 2.6+ won’t work because the underlying frameworks we instrument are shaded.
Also add server tests. We could do a lot more testing since it seems play still supports using Netty as the backing server even though it’s not the default. It’s difficult to do extensive testing though because they have so many breaking changes between versions.
The guava cache used internally wasn’t cleaning (releasing references to) the expired entries properly, resulting in excessive memory overhead.
This PR also increases the size of the cache but reduces the last used expiration window.
I also added some tests to verify the expected behavior of the cache.
Tyler Benson
Laplie Anderson
Laplie Anderson
dougqh
Nikolay Martynov
Ryan Fitzpatrick
Marco Ferrer
Douglas Q Hawkins
Luca Abbati