SkyAPM / go2sky

Distributed tracing and monitor SDK in Go for Apache SkyWalking APM
https://skywalking.apache.org/
Apache License 2.0
448 stars 122 forks source link

add golang application log support #173

Closed chwjbn closed 1 year ago

chwjbn commented 1 year ago

support for application log send to aop

wu-sheng commented 1 year ago

Why do we send agent log to backend? How should we separate them from original logs?

chwjbn commented 1 year ago

Why do we send agent log to backend? How should we separate them from original logs?

sorry,we can use grpc reporter to send golang application log from agent to backend,it's realized in java application

wu-sheng commented 1 year ago

Why do we send agent log to backend? How should we separate them from original logs?

sorry,we can use grpc reporter to send golang application log from agent to backend,it's realized in java application

Please update docs to show how to do this.

wu-sheng commented 1 year ago

Sending agent log is different from sending application logs. Please be clear about this PR purpose.

wu-sheng commented 1 year ago

The doc seems not updated yet.

chwjbn commented 1 year ago

The doc seems not updated yet.

The doc has updated now

image

wu-sheng commented 1 year ago

CI fails, please fix them.

wu-sheng commented 1 year ago

Also, you should add a new plugin test. Take this trace plugin as an example, https://github.com/SkyAPM/go2sky/tree/master/plugins/http

You could verify logs this way, https://skywalking.apache.org/docs/skywalking-java/next/en/setup/service-agent/java-agent/plugin-test/#expecteddatayaml (Expected Data Format Of The Log Items)

wu-sheng commented 1 year ago

Also, you should add a new plugin test. Take this trace plugin as an example, https://github.com/SkyAPM/go2sky/tree/master/plugins/http

You could verify logs this way, https://skywalking.apache.org/docs/skywalking-java/next/en/setup/service-agent/java-agent/plugin-test/#expecteddatayaml (Expected Data Format Of The Log Items)

Please prepare this as well. We need to verify features through e2e.

wu-sheng commented 1 year ago

The build seems failed, please fix the CI.

wu-sheng commented 1 year ago

I updated the docs for you. The previous format and naming are still little strange.

wu-sheng commented 1 year ago

You may need to fix CI.

wu-sheng commented 1 year ago

Still need some fixes of e2e.

codecov-commenter commented 1 year ago

Codecov Report

Merging #173 (b5d22d7) into master (50a1482) will decrease coverage by 4.07%. The diff coverage is 4.30%.

@@            Coverage Diff             @@
##           master     #173      +/-   ##
==========================================
- Coverage   61.85%   57.77%   -4.08%     
==========================================
  Files          23       24       +1     
  Lines        1219     1312      +93     
==========================================
+ Hits          754      758       +4     
- Misses        405      493      +88     
- Partials       60       61       +1     
Impacted Files Coverage Δ
log.go 0.00% <0.00%> (ø)
reporter/log.go 0.00% <0.00%> (ø)
trace.go 81.37% <ø> (ø)
reporter/grpc.go 43.34% <6.45%> (-7.86%) :arrow_down:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

wu-sheng commented 1 year ago

Tests are still failing.

wu-sheng commented 1 year ago

It is better you verified e2e locally first.

chwjbn commented 1 year ago

yes,this is my first pr for the project,I studied the full e2e doc and project