Skip to content

Fix the bug that can't query JVM instance metrics#79

Merged
fgksgf merged 4 commits intoapache:masterfrom
fgksgf:fix
Dec 10, 2020
Merged

Fix the bug that can't query JVM instance metrics#79
fgksgf merged 4 commits intoapache:masterfrom
fgksgf:fix

Conversation

@fgksgf
Copy link
Copy Markdown
Member

@fgksgf fgksgf commented Dec 8, 2020

Because the demo env have changed, I can't test CLI commands now.

Resolve apache/skywalking#5973

@fgksgf fgksgf added the bug Something isn't working label Dec 8, 2020
@fgksgf fgksgf added this to the 0.6.0 milestone Dec 8, 2020
@fgksgf fgksgf marked this pull request as draft December 8, 2020 09:49
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.

FYI, other reviewers, I am arguing the solution here, apache/skywalking#5973 (comment)

@fgksgf
Copy link
Copy Markdown
Member Author

fgksgf commented Dec 8, 2020

I will let users specify the scope.

@fgksgf fgksgf marked this pull request as ready for review December 9, 2020 02:56
@fgksgf fgksgf requested review from kezhenxu94 and wu-sheng December 9, 2020 02:57
@codecov-io
Copy link
Copy Markdown

Codecov Report

Merging #79 (f670e6c) into master (8b31c49) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #79   +/-   ##
=======================================
  Coverage   51.56%   51.56%           
=======================================
  Files           9        9           
  Lines         128      128           
=======================================
  Hits           66       66           
  Misses         54       54           
  Partials        8        8           
Impacted Files Coverage Δ
commands/interceptor/scope.go 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8b31c49...f670e6c. Read the comment docs.

kezhenxu94
kezhenxu94 previously approved these changes Dec 9, 2020
Copy link
Copy Markdown
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.

LGTM

@kezhenxu94 kezhenxu94 added the don't merge Don't merge for now for some reasons label Dec 9, 2020
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 confirm whether remove don't merge.

@fgksgf
Copy link
Copy Markdown
Member Author

fgksgf commented Dec 10, 2020

LGTM. Please confirm whether remove don't merge.

Tested locally, it works well.

@fgksgf fgksgf removed the don't merge Don't merge for now for some reasons label Dec 10, 2020
@fgksgf fgksgf merged commit 2b7fae8 into apache:master Dec 10, 2020
@fgksgf fgksgf deleted the fix branch December 10, 2020 01:14
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.

[CLI]Failed to query JVM instance metrics

4 participants