Skip to content

Optimize ElasticSearch 6.x 7.x plugin compatibility#607

Merged
wu-sheng merged 18 commits intoapache:mainfrom
shichaoyuan:bugfix-elasticsearch-v7
Sep 12, 2023
Merged

Optimize ElasticSearch 6.x 7.x plugin compatibility#607
wu-sheng merged 18 commits intoapache:mainfrom
shichaoyuan:bugfix-elasticsearch-v7

Conversation

@shichaoyuan
Copy link
Copy Markdown
Contributor

The parameters of the analyze method are different between 6.x and 7.x versions:

  • 6.x org.elasticsearch.action.admin.indices.analyze.AnalyzeRequest
  • 7.x org.elasticsearch.client.indices.AnalyzeRequest

@shichaoyuan shichaoyuan changed the title Fix eelasticsearch-6.x-plugin IndicesClientAnalyzeMethodsInterceptor java.lang.NoClassDefFoundError Fix elasticsearch-6.x-plugin IndicesClientAnalyzeMethodsInterceptor java.lang.NoClassDefFoundError Sep 6, 2023
@wu-sheng
Copy link
Copy Markdown
Member

wu-sheng commented Sep 6, 2023

Could you update the test scenarios covering this case?

@wu-sheng wu-sheng added the bug Something isn't working label Sep 6, 2023
@wu-sheng wu-sheng added this to the 9.1.0 milestone Sep 6, 2023
@shichaoyuan
Copy link
Copy Markdown
Contributor Author

Could you update the test scenarios covering this case?

add function test in elasticsearch-7.x-scenario

@wu-sheng
Copy link
Copy Markdown
Member

wu-sheng commented Sep 7, 2023

It seems the test fails on elasticsearch-7 case, please recheck.

https://github.com/apache/skywalking-java/actions/runs/6107504935/job/16576014494?pr=607

@shichaoyuan
Copy link
Copy Markdown
Contributor Author

It seems the test fails on elasticsearch-7 case, please recheck.

https://github.com/apache/skywalking-java/actions/runs/6107504935/job/16576014494?pr=607

fixed

@wu-sheng
Copy link
Copy Markdown
Member

wu-sheng commented Sep 7, 2023

Still failing.

@shichaoyuan
Copy link
Copy Markdown
Contributor Author

Still failing.

https://github.com/apache/skywalking-java/blob/bc7447a2c2de1c6f11ba8bd6f35dd64a2c474746/test/plugin/scenarios/elasticsearch-7.x-scenario/src/main/java/org/apache/skywalking/apm/testcase/elasticsearch/controller/CaseController.java#L43C5-L46

    public String healthcheck() throws Exception {
        restHighLevelClientCase.healthCheck();
        return "Success";
    }

It looks like healthcheck is mixed into the result check.

I think restHighLevelClientCase.healthCheck() can be removed

@wu-sheng
Copy link
Copy Markdown
Member

wu-sheng commented Sep 8, 2023

Healthcheck is for determining the app and dependencies are standing by. I think it is necessary. What is the cause of this failing?

@shichaoyuan
Copy link
Copy Markdown
Contributor Author

Healthcheck is for determining the app and dependencies are standing by. I think it is necessary. What is the cause of this failing?

actual data: {
    "segmentItems": [
      {
        "serviceName": "elasticsearch-7.x-scenario",
        "segmentSize": "2",
        "segments": [
          {
            "segmentId": "1e84afbcaf3541af8df1be94b22cd050.47.16940869262530000",
            "spans": [
              {
                "operationName": "Elasticsearch/Health",
                "parentSpanId": "0",
                "spanId": "1",
                "spanLayer": "Database",
                "tags": [
                  {
                    "key": "db.type",
                    "value": "Elasticsearch"
                  }
                ],
                "startTime": "1694086926351",
                "endTime": "1694086926610",
                "componentId": "77",
                "isError": "false",
                "spanType": "Exit",
                "peer": "elasticsearch-server-7.x:9200",
                "skipAnalysis": "false"
              },
              {
                "operationName": "HEAD:/elasticsearch-case/case/healthCheck",
                "parentSpanId": "-1",
                "spanId": "0",
                "spanLayer": "Http",
                "tags": [
                  {
                    "key": "url",
                    "value": "http://localhost:8080/elasticsearch-case/case/healthCheck"
                  },
                  {
                    "key": "http.method",
                    "value": "HEAD"
                  },
                  {
                    "key": "http.status_code",
                    "value": "200"
                  }
                ],
                "startTime": "1694086926255",
                "endTime": "1694086926638",
                "componentId": "1",
                "isError": "false",
                "spanType": "Entry",
                "peer": "",
                "skipAnalysis": "false"
              }
            ]
          },
          {
            "segmentId": "1e84afbcaf3541af8df1be94b22cd050.34.16940869273440000",
            "spans": [
              {
                "operationName": "/76b8e333-ed41-42e7-bac8-fc44b3b53088/_refresh?ignore_throttled\u003dfalse\u0026ignore_unavailable\u003dfalse\u0026expand_wildcards\u003dopen\u0026allow_no_indices\u003dtrue",
                "parentSpanId": "0",
                "spanId": "1",
                "spanLayer": "Http",
                "tags": [
                  {
                    "key": "url",
                    "value": "/76b8e333-ed41-42e7-bac8-fc44b3b53088/_refresh?ignore_throttled\u003dfalse\u0026ignore_unavailable\u003dfalse\u0026expand_wildcards\u003dopen\u0026allow_no_indices\u003dtrue"
                  },
                  {
                    "key": "http.method",
                    "value": "POST"
                  },
                  {
                    "key": "http.status_code",
                    "value": "200"
                  }
                ],
                "startTime": "1694086927346",
                "endTime": "1694086927453",
                "componentId": "26",
                "isError": "true",
                "spanType": "Exit",
                "peer": "elasticsearch-server-7.x:9200",
                "skipAnalysis": "false"
              },
              {
                "operationName": "HttpAsyncClient/local",
                "parentSpanId": "-1",
                "spanId": "0",
                "spanLayer": "Http",
                "refs": [
                  {
                    "parentEndpoint": "GET:/elasticsearch-case/case/elasticsearch",
                    "networkAddress": "",
                    "refType": "CrossThread",
                    "parentSpanId": 0,
                    "parentTraceSegmentId": "1e84afbcaf3541af8df1be94b22cd050.48.16940869266830000",
                    "parentServiceInstance": "f07a08c04[799](https://github.com/apache/skywalking-java/actions/runs/6109031224/job/16579783820#step:4:805)42e3bad96604216178ad@172.18.0.3",
                    "parentService": "elasticsearch-7.x-scenario",
                    "traceId": "1e84afbcaf3541af8df1be94b22cd050.48.16940869266830001"
                  }
                ],
                "startTime": "1694086927344",
                "endTime": "1694086927453",
                "componentId": "26",
                "isError": "false",
                "spanType": "Local",
                "peer": "",
                "skipAnalysis": "false"
              }
            ]
          }
        ]
      }
    ],

It seems that there is an abnormal "operationName": "HttpAsyncClient/local"

@wu-sheng
Copy link
Copy Markdown
Member

wu-sheng commented Sep 8, 2023

This is something I can't directly tell the reason. If it exists, it is traced. So, maybe some parts of the codes are using or calling HttpAsyncClient? It indicates a cross-thread ref, so is there something(context snapshot) that is read and continues?
I think you have to debug the codes.

@shichaoyuan
Copy link
Copy Markdown
Contributor Author

image

Found the reason.

The refresh method is not enhanced in the ElasticSearch plugin.

Let me deal with the refresh method

@shichaoyuan
Copy link
Copy Markdown
Contributor Author

image
An exception is thrown here, I will fix it again

@wu-sheng
Copy link
Copy Markdown
Member

wu-sheng commented Sep 8, 2023

This could be an illegal string?

@shichaoyuan
Copy link
Copy Markdown
Contributor Author

This could be an illegal string?

the BytesReference returned by mappings() method was changed.

7.3


public abstract class BytesReference implements Comparable<BytesReference>, ToXContentFragment 

7.17

package org.elasticsearch.common.bytes;
public interface BytesReference extends Comparable<BytesReference>, ToXContentFragment 

@wu-sheng
Copy link
Copy Markdown
Member

wu-sheng commented Sep 8, 2023

Are these new instrumentations or new versions you are adding to the tests? This seems to be 7.x relative issues, and the title of the PR indicates 6.x plugin bug. Could you update the title and change logs accordingly?

@shichaoyuan
Copy link
Copy Markdown
Contributor Author

Are these new instrumentations or new versions you are adding to the tests? This seems to be 7.x relative issues, and the title of the PR indicates 6.x plugin bug. Could you update the title and change logs accordingly?

image

added more versions in support-version.list

@shichaoyuan shichaoyuan changed the title Fix elasticsearch-6.x-plugin IndicesClientAnalyzeMethodsInterceptor java.lang.NoClassDefFoundError Optimize ElasticSearch 6.x 7.x plugin compatibility Sep 8, 2023
@wu-sheng
Copy link
Copy Markdown
Member

wu-sheng commented Sep 8, 2023

Still have errors.

@shichaoyuan
Copy link
Copy Markdown
Contributor Author

Still have errors.

  Pulling elasticsearch-server-7.x (elasticsearch:7.10.2)...
  manifest for elasticsearch:7.10.2 not found: manifest unknown: manifest unknown
  docker startup failure!
  Error response from daemon: No such image: elasticsearch:7.10.2
  Error: Process completed with exit code 1.

Let me see where the image was pulled from.

@wu-sheng
Copy link
Copy Markdown
Member

I remember Elasticsearch 7.10 changed the license, so maybe it needs to download from a new docker hub repo? If so, you have to separate the test cases

Comment on lines +37 to +38
<dependencies>
</dependencies>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this is not necessary.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

removed

Comment on lines +33 to +35
<properties>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
</properties>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we need this?

Comment on lines +79 to +87
EnhancedInstance actions;
if (allArguments.length > 2) {
// (Settings settings, GenericAction<Request, Response> action, TransportService transportService)
actions = (EnhancedInstance) allArguments[2];
} else {
// 7.11 +
// (ActionType<Response> action, TransportService transportService)
actions = (EnhancedInstance) allArguments[1];
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is not a proper way to fix this, please use different interceptors with different instrumentations.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed

@shichaoyuan
Copy link
Copy Markdown
Contributor Author

The XContentBuilder moved the package location in version 7.16, and I fixed the test cases again.

@shichaoyuan
Copy link
Copy Markdown
Contributor Author

image
image
This image exists, it may be due to an occasional pull failure.

@wu-sheng
Copy link
Copy Markdown
Member

I tried to re-run, if you pull too many versions of images, I have concerns whether the disk volume in the CI env is enough.

@wu-sheng wu-sheng merged commit 44b8f66 into apache:main Sep 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants