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

Parse the Redis optional replication section redis_replication measuremet #5921

Merged
merged 1 commit into from Sep 12, 2019

Conversation

@adamflott
Copy link
Contributor

adamflott commented May 29, 2019

Parse the Redis replication nodes into individual pieces (ip, port, offset, lag) instead of a single string

Required for all PRs:

  • Signed CLA.
  • [N/A] Associated README.md updated.
  • Has appropriate unit tests.
Copy link
Member

glinton left a comment

Would you document the new redis_replication measurement in the readme? thanks

@adamflott

This comment has been minimized.

Copy link
Contributor Author

adamflott commented Aug 15, 2019

@glinton done

plugins/inputs/redis/redis.go Outdated Show resolved Hide resolved
plugins/inputs/redis/redis.go Outdated Show resolved Hide resolved
plugins/inputs/redis/redis.go Outdated Show resolved Hide resolved
@danielnelson danielnelson added this to the 1.12.0 milestone Aug 15, 2019
Copy link
Member

glinton left a comment

Thanks, can you also update theses to use the new format:

### Metrics
- measurement1
  - tags:
    - tag1 (optional description)
    - tag2
  - fields:
    - field1 (type, unit)
    - field2 (float, percent)

+ measurement2
@danielnelson danielnelson modified the milestones: 1.12.0, 1.13.0 Aug 20, 2019
@adamflott adamflott force-pushed the adamflott:redis-parse-replication branch from a773cd9 to e8506e2 Aug 26, 2019
@adamflott

This comment has been minimized.

Copy link
Contributor Author

adamflott commented Aug 26, 2019

@danielnelson @glinton I believe I addressed all the feedback, please review.

@glinton

This comment has been minimized.

Copy link
Member

glinton commented Aug 26, 2019

In the future, once a review has started, please refrain from force-pushing as we don't always have time for a compete re-review. thanks

@adamflott

This comment has been minimized.

Copy link
Contributor Author

adamflott commented Sep 12, 2019

Any update? The code is ready. Also, is 1.13 the appropriate target? I feel this is a minor change and could likely be included in the 1.12.x line

@danielnelson danielnelson merged commit 57f58fd into influxdata:master Sep 12, 2019
7 checks passed
7 checks passed
ci/circleci: deps Your tests passed on CircleCI!
Details
ci/circleci: package Your tests passed on CircleCI!
Details
ci/circleci: test-go-1.10 Your tests passed on CircleCI!
Details
ci/circleci: test-go-1.11 Your tests passed on CircleCI!
Details
ci/circleci: test-go-1.12 Your tests passed on CircleCI!
Details
ci/circleci: test-go-1.12-386 Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@danielnelson

This comment has been minimized.

Copy link
Contributor

danielnelson commented Sep 12, 2019

Thanks @adamflott, sorry this will need to go out in 1.13 since we try to make as few changes as possible to the release branches to avoid introducing new bugs.

bitcharmer added a commit to bitcharmer/telegraf that referenced this pull request Oct 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.