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

Support for Elastic.Clients.Elasticsearch #1935

Merged

Conversation

gregkalapos
Copy link
Contributor

@gregkalapos gregkalapos commented Dec 5, 2022

This PR is covering the rest of the things needed to support the new Elasticsearch .NET Client.

The core of the change was done directly in the client code: elastic/elasticsearch-net#6990

This PR adds:

  • Integration-tests with docker by using the new es client and by using the OTel bridge in the agent.
  • Implements suppression of HTTP span creation when elasticsearch spans are already created by the OTel bridge.

Regarding release: I think this should not block a release - if this ends up being merged after the next release, it's still fine.

@gregkalapos gregkalapos self-assigned this Dec 5, 2022
@gregkalapos gregkalapos requested a review from z1c0 December 5, 2022 08:31
@apmmachine
Copy link
Contributor

apmmachine commented Dec 5, 2022

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2022-12-07T15:45:27.770+0000

  • Duration: 84 min 13 sec

Test stats 🧪

Test Results
Failed 0
Passed 22143
Skipped 175
Total 22318

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • run benchmark tests : Run the benchmark test.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@gregkalapos
Copy link
Contributor Author

gregkalapos commented Dec 5, 2022

Looking at

[2022-12-05T08:48:41.400Z] /var/lib/jenkins/workspace/net_apm-agent-dotnet-mbp_PR-1935/apm-agent-dotnet/src/Elastic.Apm/OpenTelemetry/ElasticSearchHttpNonTracer.cs(29,35): error CS1061: 'Activity' does not contain a definition for 'DisplayName' and no accessible extension method 'DisplayName' accepting a first argument of type 'Activity' could be found (are you missing a using directive or an assembly reference?) [/var/lib/jenkins/workspace/net_apm-agent-dotnet-mbp_PR-1935/apm-agent-dotnet/src/Elastic.Apm/Elastic.Apm.csproj::TargetFramework=netstandard2.0]

Update: fixed.

gregkalapos and others added 5 commits December 6, 2022 21:07
Copy link
Contributor

@z1c0 z1c0 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@gregkalapos gregkalapos merged commit a81ac60 into elastic:main Dec 7, 2022
@gregkalapos gregkalapos deleted the Clients_ElasticSearch_Integration branch December 7, 2022 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants