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

fix: mongodb input plugin issue #9845 #9846

Merged
merged 3 commits into from
Oct 4, 2021
Merged

Conversation

howardyoo
Copy link
Contributor

input plugin would fail (and result in seg fault) because there was no check to see if newStat.Metrics.Repl.Network.GetMores was nil or not. Added a checking so that the assignment would happen only when newStat.Metrics.Repl.Network.GetMores is not nil.

Required for all PRs:

  • Updated associated README.md.
  • Wrote appropriate unit tests.
  • Pull request title or commits are in conventional commit format (e.g. feat: or fix:)

resolves #9845

This code change will make sure to check the newStat.Metrics.Repl.Network.GetMores is not null, before performing any access to its underlying variables. This will prevent the mongodb plugin from resulting into segmentation violation that the issue #9845 reported.

if newStat.Metrics.Repl.Network.GetMores != nil {
					returnVal.ReplNetworkGetmoresNum = newStat.Metrics.Repl.Network.GetMores.Num
					returnVal.ReplNetworkGetmoresTotalMillis = newStat.Metrics.Repl.Network.GetMores.TotalMillis
				}

input plugin would fail (and result in seg fault) because there was no check to see if newStat.Metrics.Repl.Network.GetMores was nil or not. Added a checking so that the assignment would happen only when newStat.Metrics.Repl.Network.GetMores is not nil.

### Required for all PRs:

<!-- Complete the tasks in the following list. Change [ ] to [x] to
show completion. -->

- [ ] Updated associated README.md.
- [ ] Wrote appropriate unit tests.
- [x] Pull request title or commits are in [conventional commit format](https://www.conventionalcommits.org/en/v1.0.0/#summary) (e.g. feat: or fix:)

<!-- Link to issues that describe the need for the change. Issues
should include context that will help reviewers understand why the
change is needed.

Make sure to link issues and using a keyword like "resolves influxdata#1234".
https://docs.github.com/en/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword
-->

resolves influxdata#9845

<!-- Finally, include a summary of the code change itself. This
description should tell reviewers how the issues were resolved.
example: Added an input plugin to gather yak shaving metrics using
golang library yaktech/shaver. -->

This code change will make sure to check the newStat.Metrics.Repl.Network.GetMores is not null, before performing any access to its underlying variables. This will prevent the mongodb plugin from resulting into segmentation violation that the issue influxdata#9845 reported.

```
if newStat.Metrics.Repl.Network.GetMores != nil {
					returnVal.ReplNetworkGetmoresNum = newStat.Metrics.Repl.Network.GetMores.Num
					returnVal.ReplNetworkGetmoresTotalMillis = newStat.Metrics.Repl.Network.GetMores.TotalMillis
				}
```
@telegraf-tiger
Copy link
Contributor

telegraf-tiger bot commented Oct 1, 2021

Thanks so much for the pull request!
🤝 ✒️ Just a reminder that the CLA has not yet been signed, and we'll need it before merging. Please sign the CLA when you get a chance, then post a comment here saying !signed-cla

@telegraf-tiger telegraf-tiger bot added the feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin label Oct 1, 2021
@howardyoo
Copy link
Contributor Author

!signed-cla

Copy link
Contributor Author

@howardyoo howardyoo left a comment

Choose a reason for hiding this comment

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

ran make fmt to format the file appropriately.

Copy link
Member

@helenosheaa helenosheaa left a comment

Choose a reason for hiding this comment

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

lgtm, thanks for the PR

@helenosheaa helenosheaa added the ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review. label Oct 4, 2021
@powersj powersj merged commit c1f51b0 into influxdata:master Oct 4, 2021
reimda pushed a commit that referenced this pull request Oct 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

mongodb input plugin will crash telegraf
3 participants