Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PMM-1901 metrics endpoint and collectors filtering. #23

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

idexter
Copy link

@idexter idexter commented Jul 16, 2019

No description provided.

@CLAassistant
Copy link

CLAassistant commented Jul 16, 2019

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Anton Kucherov seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

basic/basic.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
basic/basic.go Outdated Show resolved Hide resolved
@idexter idexter requested review from AlekSi and BupycHuk July 16, 2019 17:01
@codecov
Copy link

codecov bot commented Jul 16, 2019

Codecov Report

Merging #23 into master will decrease coverage by 0.45%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #23      +/-   ##
==========================================
- Coverage   83.71%   83.26%   -0.46%     
==========================================
  Files           9        9              
  Lines         694      699       +5     
==========================================
+ Hits          581      582       +1     
- Misses        100      103       +3     
- Partials       13       14       +1
Impacted Files Coverage Δ
basic/basic.go 100% <100%> (ø) ⬆️
client/transport.go 87.5% <0%> (-12.5%) ⬇️

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 2df9583...0f01209. Read the comment docs.

@rnovikovP
Copy link

@dexterHD pls click CLA button

main.go Outdated Show resolved Hide resolved
basic/metrics.go Outdated Show resolved Hide resolved
@idexter idexter requested a review from BupycHuk July 19, 2019 16:01
BupycHuk
BupycHuk previously approved these changes Jul 30, 2019
factory/factory.go Outdated Show resolved Hide resolved
BupycHuk
BupycHuk previously approved these changes Jul 30, 2019
basic/basic.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
@idexter idexter requested review from AlekSi and BupycHuk July 31, 2019 16:41
@idexter
Copy link
Author

idexter commented Aug 13, 2019

@AlekSi Please, check this out when you have a time. It's waiting for 2 weeks.

@AlekSi
Copy link

AlekSi commented Aug 13, 2019

rds_exporter is not used by PMM 2.0 beta6, so it is not a priority.

That being said, if you want to do something about this PR, please think how to simplify factory.go. Code there looks overcomplicated for an apparently simple task.

@idexter
Copy link
Author

idexter commented Aug 13, 2019

@rnovikovP @AlekSi But it's in PMM 2.0 beta6 release in JIRA 😕 As about complicated code, it's because of backward compatibility with the old endpoints. I'm not sure it worth to do something about that in the meaning of priorities.

@AlekSi AlekSi self-assigned this Dec 11, 2019
@idexter idexter closed this Feb 11, 2020
@AlekSi AlekSi reopened this Feb 12, 2020
@AlekSi AlekSi removed their request for review February 17, 2021 08:31
@AlekSi AlekSi removed their assignment Jan 29, 2022
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.

6 participants