Skip to content

Adapt the new v3 protocol#30

Merged
wu-sheng merged 21 commits intoapache:masterfrom
mrproliu:v8
Apr 9, 2020
Merged

Adapt the new v3 protocol#30
wu-sheng merged 21 commits intoapache:masterfrom
mrproliu:v8

Conversation

@mrproliu
Copy link
Copy Markdown
Contributor

@mrproliu mrproliu commented Apr 4, 2020

Follow the new v3 protocol, I rewrite the Nginx agent.
Currently, I'm still waiting for the backend to finish, and then I can carry out the E2E test.

@mrproliu mrproliu requested review from dmsolr, moonming and wu-sheng April 4, 2020 07:46
@mrproliu mrproliu added this to the 0.2.0 milestone Apr 4, 2020
@mrproliu mrproliu added the enhancement New feature or request label Apr 4, 2020
Comment thread README.md
Comment thread README.md Outdated
Comment thread README.md Outdated
Comment thread lib/skywalking/client.lua Outdated
Comment thread lib/skywalking/client.lua Outdated
Copy link
Copy Markdown
Member

@wu-sheng wu-sheng left a comment

Choose a reason for hiding this comment

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

LGTM. Please test this with 8.0 core backend and java agent. They are ready to test.

@wu-sheng
Copy link
Copy Markdown
Member

wu-sheng commented Apr 7, 2020

At apache/skywalking#4599, all java plugin tests are passed. And we are just waiting for @kezhenxu94 updating the e2e test result.

@mrproliu
Copy link
Copy Markdown
Contributor Author

mrproliu commented Apr 7, 2020

LGTM. Please test this with 8.0 core backend and java agent. They are ready to test.

Sure.

@wu-sheng
Copy link
Copy Markdown
Member

wu-sheng commented Apr 7, 2020

And test results should feedback to mail [8.0.0] Move forward to 8.0.0. REMOVE the register and inventory permanently

Comment thread lib/skywalking/client.lua Outdated
Comment thread lib/skywalking/client.lua Outdated
Comment thread lib/skywalking/client.lua Outdated
@mrproliu
Copy link
Copy Markdown
Contributor Author

mrproliu commented Apr 7, 2020

@membphis Thanks for your review.

Comment thread lib/skywalking/client.lua Outdated
Comment thread lib/skywalking/management.lua Outdated
Comment thread lib/skywalking/management.lua Outdated
Comment thread lib/skywalking/segment_ref.lua Outdated
Comment thread lib/skywalking/util.lua Outdated
Comment thread lib/skywalking/util.lua Outdated
Comment on lines +83 to +84
local newID = function()
return timestamp() .. '.' .. math.random(0, MAX_ID_PART2) .. '.' .. math.random(0, MAX_ID_PART3)
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 function will always run, which is redundant in an OpenResty environment

Comment thread lib/skywalking/util.lua
@moonming
Copy link
Copy Markdown
Member

moonming commented Apr 7, 2020

one more thing, is it works with the current version of skywalking? Or we should merge this PR after skywalking 8.0 released?

@wu-sheng
Copy link
Copy Markdown
Member

wu-sheng commented Apr 7, 2020

one more thing, is it works with the current version of skywalking? Or we should merge this PR after skywalking 8.0 released?

It isn't compatible with old SkyWalking release.

@mrproliu please add some documentation in this section, https://github.com/apache/skywalking-nginx-lua#download, such as

  • 0.1.0 release requires SkyWalking 7
  • 0.2.0+ releases require SkyWalking 8

@moonming
Copy link
Copy Markdown
Member

moonming commented Apr 7, 2020 via email

@wu-sheng wu-sheng requested a review from moonming April 8, 2020 12:24
Comment thread lib/skywalking/tracer.lua
tracingContext = TC.new(serviceName, serviceInstanceName)

-- Constant pre-defined in SkyWalking main repo
-- 84 represents Nginx
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 a previous wrong comment, please fix this later.

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.

Send a separated PR for this, I am merging this one.

@wu-sheng wu-sheng merged commit d754b89 into apache:master Apr 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants