8000 Fix Elasticsearch function test by sebastienros · Pull Request #7547 · dotnet/aspire · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix Elasticsearch function test #7547

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Feb 13, 2025
Merged

Fix Elasticsearch function test #7547

merged 1 commit into from
Feb 13, 2025

Conversation

sebastienros
Copy link
Member

Description

Fixes #7452

Checklist

  • Is this feature complete?
    • Yes. Ready to ship.
    • No. Follow-up changes expected.
  • Are you including unit tests for the changes and scenario tests if relevant?
    • Yes
    • No
  • Did you add public API?
    • Yes
      • If yes, did you have an API Review for it?
        • Yes
        • No
      • Did you add <remarks /> and <code /> elements on your triple slash comments?
        • Yes
        • No
    • No
  • Does the change make any security assumptions or guarantees?
    • Yes
      • If yes, have you done a threat model and had a security review?
        • Yes
        • No
    • No
  • Does the change require an update in our Aspire docs?

@sebastienros
Copy link
Member Author

@Mpdreamz does this change make sense? The PingRequest uses "ping" as the OperationName, I assume the otel part of ES is doing this projection to DisplayName?

@stevejgordon
Copy link

@sebastienros I'm going to check this on our end. I expected both DisplayName and OperationName to match and be "ping", so there may be a bug in a recent refactoring of the OTel instrumentation in our libraries.

@eerhardt eerhardt mentioned this pull request Feb 13, 2025
Closed
@stevejgordon
Copy link

@sebastienros - Having reviewed our changes, I see that this is expected, and this PR LGTM.

Our Transport layer creates a general Activity passing in the HTTP method name, which becomes OperationName and DisplayName (HEAD in this case). We provide a callback to our client library built on top of Transport, which then sets the DisplayName to the logical operation name (ping in this case). As there is no API to change OperationName, we leave that as is. Since OTLP exports using the DisplayName, this is fine.

@davidfowl
Copy link
Member

Thanks @stevejgordon !

@davidfowl davidfowl merged commit d25bb1f into main Feb 13, 2025
70 checks passed
@davidfowl davidfowl deleted the sebros/elastictest branch February 13, 2025 08:34
@github-actions github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Mar 10, 2025
@github-actions github-actions bot locked and limited conversation to collaborators Apr 9, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@davidfowl davidfowl davidfowl approved these changes

Assignees
No one assigned
Labels
needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Test] Fix ElasticSearch client integration tests on GitHub Actions
3 participants
@sebastienros @stevejgordon @davidfowl
0