Skip to content

add e2e test for kafka transporter#42

Merged
wu-sheng merged 21 commits intoapache:mainfrom
dmsolr:e2e/kafka
Oct 18, 2021
Merged

add e2e test for kafka transporter#42
wu-sheng merged 21 commits intoapache:mainfrom
dmsolr:e2e/kafka

Conversation

@dmsolr
Copy link
Member

@dmsolr dmsolr commented Oct 3, 2021

  • If this pull request closes/resolves/fixes an existing issue, replace the issue number. Closes #.
  • Update the CHANGES log.

# limitations under the License.

logs:
{{- range .logs }}
Copy link
Member

Choose a reason for hiding this comment

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

Same issue as apache/skywalking#7849 (comment), please read that thread.

Copy link
Member

Choose a reason for hiding this comment

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

This is not resolved yet

@dmsolr
Copy link
Member Author

dmsolr commented Oct 3, 2021

Do we have any approach to set docker-compose file by the environment to reduce copy-codes? @kezhenxu94

@kezhenxu94
Copy link
Member

Do we have any approach to set docker-compose file by the environment to reduce copy-codes? @kezhenxu94

Not now, I'm also considering to provide this feature

@wu-sheng wu-sheng requested a review from kezhenxu94 October 10, 2021 10:03
@wu-sheng wu-sheng added this to the 8.8.0 milestone Oct 10, 2021
@wu-sheng
Copy link
Member

https://neo4j.com/docs/java-manual/current

Please fixe dead link.

# limitations under the License.

spans:
{{- range .spans}}
Copy link
Member

Choose a reason for hiding this comment

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

range should be replaced by contains. We have similar suggestions in the main repo. Please check.

Copy link
Member

@kezhenxu94 kezhenxu94 left a comment

Choose a reason for hiding this comment

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

https://neo4j.com/docs/java-manual/current

Please fixe dead link.

@dmsolr should be https://neo4j.com/docs/java-manual/current/ (note the tailing /), I update this in another PR but it is not ready yet #41

matrix:
case:
- name: gRPC
path: test/e2e/case/base/e2e.yaml
Copy link
Member

Choose a reason for hiding this comment

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

What about changing this to test/e2e/case/grpc/e2e.yaml?

- query: swctl --display yaml --base-url=http://${oap_host}:${oap_12800}/graphql trace ls
expected: expected/traces-list.yml
# native tracing: trace detail
- query: swctl --display yaml --base-url=http://${oap_host}:${oap_12800}/graphql trace $(swctl --display yaml --base-url=http://${oap_host}:${oap_12800}/graphql trace ls|grep -A 5 'POST:/info'|tail -n1|awk -F ' ' '{print $2}')
Copy link
Member

Choose a reason for hiding this comment

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

Don't use grep, refer to how we do this in th main repo

expected: expected/event-list.yml

# native log: logs list
- query: swctl --display yaml --base-url=http://${oap_host}:${oap_12800}/graphql logs list --service-id=$(swctl --display yaml --base-url=http://${oap_host}:${oap_12800}/graphql service ls|grep -B 1 'provider'|yq e '.[0].id' -) --trace-id=$(swctl --display yaml --base-url=http://${oap_host}:${oap_12800}/graphql trace ls|grep -A 5 'POST:/info'|tail -n1|awk -F ' ' '{print $2}')
Copy link
Member

Choose a reason for hiding this comment

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

Same here

# limitations under the License.

events:
{{- range .events }}
Copy link
Member

Choose a reason for hiding this comment

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

s/range/contains

# limitations under the License.

logs:
{{- range .logs }}
Copy link
Member

Choose a reason for hiding this comment

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

This is not resolved yet

# See the License for the specific language governing permissions and
# limitations under the License.

{{- range .}}
Copy link
Member

Choose a reason for hiding this comment

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

here

# specific language governing permissions and limitations
# under the License.

{{- range .}}
Copy link
Member

Choose a reason for hiding this comment

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

Here

# limitations under the License.

spans:
{{- range .spans}}
Copy link
Member

Choose a reason for hiding this comment

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

Here

# limitations under the License.

traces:
{{- range .traces }}
Copy link
Member

Choose a reason for hiding this comment

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

Here

@wu-sheng wu-sheng requested a review from kezhenxu94 October 12, 2021 06:43
Comment on lines 23 to 31
{{- if eq .servicecode "e2e-service-provider" }}
{{- cintains .refs }}
- traceid: {{ notEmpty .traceid }}
parentsegmentid: {{ notEmpty .parentsegmentid }}
parentspanid: 1
type: CROSS_PROCESS
{{- end }}
{{- end }}
{{- if eq .servicecode "e2e-service-consumer" }}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
{{- if eq .servicecode "e2e-service-provider" }}
{{- cintains .refs }}
- traceid: {{ notEmpty .traceid }}
parentsegmentid: {{ notEmpty .parentsegmentid }}
parentspanid: 1
type: CROSS_PROCESS
{{- end }}
{{- end }}
{{- if eq .servicecode "e2e-service-consumer" }}
{{- if eq .servicecode "e2e-service-provider" }}
{{- cintains .refs }}
- traceid: {{ notEmpty .traceid }}
parentsegmentid: {{ notEmpty .parentsegmentid }}
parentspanid: 1
type: CROSS_PROCESS
{{- end }}
{{- end }}
{{- if eq .servicecode "e2e-service-consumer" }}

Please note, using {{- if eq .servicecode "e2e-service-provider" }} and {{- if eq .servicecode "e2e-service-consumer" }} inside contains, you actually only verify ONE of them, as soon as one of them satisfies the condition, the others won't be verified

Comment on lines 20 to 40
{{- if eq (index .endpointnames 0) "HikariCP/Connection/getConnection" }}
- HikariCP/Connection/getConnection
{{- end }}
{{- if eq (index .endpointnames 0) "HikariCP/Connection/close" }}
- HikariCP/Connection/close
{{- end }}
{{- if eq (index .endpointnames 0) "H2/JDBI/Statement/execute" }}
- H2/JDBI/Statement/execute
{{- end}}
{{- if eq (index .endpointnames 0) "H2/JDBI/Statement/executeQuery" }}
- H2/JDBI/Statement/executeQuery
{{- end}}
{{- if eq (index .endpointnames 0) "H2/JDBI/PreparedStatement/executeQuery" }}
- H2/JDBI/PreparedStatement/executeQuery
{{- end }}
{{- if eq (index .endpointnames 0) "POST:/info" }}
- POST:/info
{{- end }}
{{- if eq (index .endpointnames 0) "POST:/users" }}
- POST:/users
{{- end }}
Copy link
Member

Choose a reason for hiding this comment

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

Same here

dmsolr and others added 3 commits October 12, 2021 15:13
Co-authored-by: kezhenxu94 <kezhenxu94@apache.org>
Co-authored-by: kezhenxu94 <kezhenxu94@apache.org>
Co-authored-by: kezhenxu94 <kezhenxu94@apache.org>
dmsolr and others added 3 commits October 12, 2021 15:13

{{- range .}}
{{- contains .}}
{{- if eq .name "POST:/info" }}
Copy link
Member

Choose a reason for hiding this comment

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

Remove this please

Copy link
Member

@kezhenxu94 kezhenxu94 left a comment

Choose a reason for hiding this comment

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

@dmsolr I'd suggest you overhaul all the if statements in the expected data file, they are mostly misused

{{- contains .traces }}
- segmentid: {{ notEmpty .segmentid }}
endpointnames:
{{- if eq (index .endpointnames 0) "POST:/info" }}
Copy link
Member

Choose a reason for hiding this comment

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

This

Comment on lines 39 to 58
{{- if eq .type "Exit" }}
/info
{{- end }}
{{- if eq .type "Entry" }}
POST:/info
{{- end }}
type: {{ notEmpty .type }}
peer:
{{- if eq .type "Exit" }}
provider:9090
{{ else }}
""
{{- end }}
component:
{{- if eq .type "Exit" }}
SpringRestTemplate
{{- end }}
{{- if eq .type "Entry" }}
Tomcat
{{- end }}
Copy link
Member

Choose a reason for hiding this comment

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

These ifs

spanid: {{ .spanid }}
parentspanid: {{ .parentspanid }}
refs:
{{- if eq .servicecode "e2e-service-provider" }}
Copy link
Member

Choose a reason for hiding this comment

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

This

@wu-sheng wu-sheng requested a review from kezhenxu94 October 17, 2021 13:07
@wu-sheng wu-sheng merged commit fbdfc42 into apache:main Oct 18, 2021
wu-sheng added a commit that referenced this pull request Oct 31, 2021
* main: (28 commits)
  fix release doc (#61)
  Support Jedis' Transaction and fix duplicated enhancement (#57)
  Initialize 8.9.0 iteration (#59)
  Polish release shell and doc. (#58)
  fix rocketmq message header properties garbled characters issue (#54)
  Fix netty-socketio plugin test failure (#56)
  Add okhttp2.x plugin (#49)
  add e2e test for kafka transporter (#42)
  Fix version badge (#53)
  Add JDK17 supported declaration. (#52)
  Fix instrumentation v2 API doesn't work for constructor instrumentation. (#51)
  Add kylin jdbc plugin (#45)
  Fix version compatibility for JSON-RPC4J Plugin (#50)
  Feature add clickhouse jdbc plugin (#41)
  Update menu.yml (#48)
  Fix format. (#47)
  Add new menu for the document (#46)
  Doc: Update setup agent in kubernetes from 'containers' to 'initContainers'. (#44)
  The httpasyncclient-4.x-plugin does not take effect every time. (#40)
  Add an agent plugin to support Jackson (#39)
  ...

# Conflicts:
#	.github/workflows/plugins-test.3.yaml
#	CHANGES.md
#	docs/en/setup/service-agent/java-agent/README.md
#	docs/menu.yml
GuoHaoZai pushed a commit to GuoHaoZai/skywalking-java that referenced this pull request Apr 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants