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

Merge dd-master into master #14

Closed
wants to merge 3,412 commits into from
Closed

Merge dd-master into master #14

wants to merge 3,412 commits into from

Conversation

trask
Copy link
Member

@trask trask commented Nov 15, 2019

Resolves #3

dougqh and others added 30 commits September 25, 2019 17:05
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.
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.
Adding spring-data-elasticsearch dependency just to make sure everything plays nice together
Addressing review comments: operationName -> resourceName

This was vestige of the original SignalFX code
Per view comment, switching setResourceName -> setTag
Use tracer from field instead of GlobalTracer
Minor optimization to avoid using BigInteger if possible
Update integrations core submodule to 6.14.0
Adding setAsync as per review comment
Removing unnecessary test code left over from initial port integration
Changing directory name to spring-data-1.9 to reflect version restrictions as per review comments
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.
@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.
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.
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.
Pinning spring-kafka @ 2.2 to fix breaking changin in spring-kafka-test 2.3
randomanderson and others added 22 commits November 4, 2019 16:16
Since global tags can only be set as String.
Postfix the existing config names with `-beta` for `servlet` and `jdbc`.

To avoid any risk that the config was explicitly enabled for an unrelated reason resulting in unexpected behavior when upgrading to the latest version.
Allow decorators to parse string values on tags.
Rename config key for beta/experimental instrumentation.
This should make things more clear
OkHttp seems to be inadvertently loading the java log manager.
OkHttp seems to be inadvertently loading the java log manager.
Disable zulu8 test for time being due to test incompatibility
Excluded classes generated by Eclipse Sisu
Remove experimental jdbc and servlet integrations until further evaluation
@trask
Copy link
Member Author

trask commented Nov 15, 2019

Closing in favor of @tylerbenson merging (as DataDog representative)

@trask trask closed this Nov 15, 2019
@trask trask deleted the merge-dd-master branch November 24, 2019 23:28
bjrara pushed a commit to bjrara/opentelemetry-java-instrumentation that referenced this pull request Nov 27, 2024
fix: temporary fix for pr build failures
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.

Merge dd-master into master
7 participants