Skip to content

feat: add error log skywalking reporter#4633

Merged
spacewander merged 25 commits into
apache:masterfrom
dmsolr:sw-logger
Aug 4, 2021
Merged

feat: add error log skywalking reporter#4633
spacewander merged 25 commits into
apache:masterfrom
dmsolr:sw-logger

Conversation

@dmsolr

@dmsolr dmsolr commented Jul 20, 2021

Copy link
Copy Markdown
Member

What this PR does / why we need it:

Pre-submission checklist:

  • Did you explain what problem does this PR solve? Or what new features have been added?
  • Have you added corresponding test cases?
  • Have you modified the corresponding document?
  • Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first

his plugin is error-log-logger-like, but it sends log data to Apache SkyWalking over http.

Comment thread apisix/plugins/error-log-skywalking-logger.lua Outdated
Comment thread apisix/plugins/error-log-logger.lua Outdated
Comment thread apisix/plugins/error-log-logger.lua
Comment thread apisix/plugins/error-log-logger.lua Outdated
Comment thread apisix/plugins/error-log-logger.lua Outdated
Comment thread apisix/plugins/error-log-logger.lua Outdated
@dmsolr

dmsolr commented Jul 22, 2021

Copy link
Copy Markdown
Member Author

If these changes are ok in basically, I will update the related docs and tests.

Comment thread apisix/plugins/error-log-logger.lua
Comment thread apisix/plugins/error-log-logger.lua Outdated
Comment thread apisix/plugins/log-generator.lua Outdated
@tokers

tokers commented Jul 24, 2021

Copy link
Copy Markdown
Contributor

@dmsolr Please also replenish the documents.

Comment thread apisix/plugins/error-log-logger.lua Outdated
Comment thread t/plugin/error-log-skywalking-logger.t
Comment thread t/plugin/error-log-skywalking-logger.t Outdated
Comment thread t/plugin/error-log-skywalking-logger.t Outdated
Comment thread t/plugin/error-log-skywalking-logger.t Outdated
Comment thread apisix/plugins/error-log-logger.lua Outdated
Comment thread docs/en/latest/plugins/error-log-logger.md Outdated
Comment thread t/plugin/error-log-logger-skywalking.t Outdated
Comment thread t/plugin/error-log-logger.t
Comment thread t/plugin/error-log-logger-skywalking.t
Comment thread t/plugin/error-log-logger-skywalking.t Outdated



=== TEST 5: log an warn level message

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What information did I miss? What's the point of these tests.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This case is testing whether a warning message could be wrapper as the skywalking log format

Comment thread docs/en/latest/plugins/error-log-logger.md Outdated
Comment thread apisix/plugins/error-log-logger.lua Outdated
Comment thread t/plugin/error-log-logger-skywalking.t
Comment thread apisix/plugins/error-log-logger.lua Outdated
Comment thread apisix/plugins/error-log-logger.lua Outdated
Comment thread t/plugin/error-log-logger.t Outdated


=== TEST 9: want to reload the plugin by route
=== TEST 9: log a warn level message (schema compatibility testing)

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.

Better to add test at the end of file

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

we have resetting environment tasks at the end.

| tcp.port | integer | required | | [0,...] | Target upstream port. |
| tcp.tls | boolean | optional | false | | Control whether to perform SSL verification. |
| tcp.tls_server_name | string | optional | | | The server name for the new TLS extension SNI. |
| skywalking.endpoint_addr | string | required | http://127.0.0.1:12900/v3/logs | | the http endpoint of Skywalking. |

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.

Once you provide a default value it is no longer required.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Got it.

Comment thread docs/en/latest/plugins/error-log-logger.md Outdated
Comment thread docs/en/latest/plugins/error-log-logger.md Outdated
Comment thread docs/en/latest/plugins/error-log-logger.md Outdated
Comment thread docs/en/latest/plugins/error-log-logger.md Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants