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

Check for nil on mongo input, fix for panic. #2848

Merged
merged 1 commit into from
May 30, 2017

Conversation

snardone
Copy link
Contributor

@snardone snardone commented May 23, 2017

Upgraded to telegraf 1.3, mongo input is generating panics in our environment:

panic: interface conversion: interface {} is nil, not bool

goroutine 238 [running]:
github.com/influxdata/telegraf/plugins/inputs/mongodb.NewStatLine(0xed0b6827e, 0xc5670ba, 0x1bea540, 0xc4204700f0, 0xc42
03024a0, 0xc420b95668, 0xc4201746c0, 0xed0b68288, 0x568a97d, 0x1bea540, ...)
/home/ubuntu/telegraf-build/src/github.com/influxdata/telegraf/plugins/inputs/mongodb/mongostat.go:567 +0x1bb2
github.com/influxdata/telegraf/plugins/inputs/mongodb.(*Server).gatherData(0xc420175000, 0x1b01540, 0xc420174f40, 0x4060
01, 0x0, 0x0)
/home/ubuntu/telegraf-build/src/github.com/influxdata/telegraf/plugins/inputs/mongodb/mongodb_server.go:107 +0xd
0f
github.com/influxdata/telegraf/plugins/inputs/mongodb.(*MongoDB).gatherServer(0xc42013c080, 0xc420175000, 0x1b01540, 0xc
420174f40, 0x1144780, 0xc420402e40)
/home/ubuntu/telegraf-build/src/github.com/influxdata/telegraf/plugins/inputs/mongodb/mongodb.go:163 +0x89
github.com/influxdata/telegraf/plugins/inputs/mongodb.(*MongoDB).Gather.func1(0xc421025230, 0x1b01540, 0xc420174f40, 0xc
42013c080, 0xc420175000)
/home/ubuntu/telegraf-build/src/github.com/influxdata/telegraf/plugins/inputs/mongodb/mongodb.go:91 +0x73
created by github.com/influxdata/telegraf/plugins/inputs/mongodb.(*MongoDB).Gather
/home/ubuntu/telegraf-build/src/github.com/influxdata/telegraf/plugins/inputs/mongodb/mongodb.go:92 +0x352

============================== 05/23/2017 15:20:50

PR resolves the issue for us. Thanks.

Required for all PRs:

  • Signed CLA.
  • Associated README.md updated.
  • Has appropriate unit tests.

@danielnelson danielnelson added this to the 1.3.1 milestone May 23, 2017
@danielnelson
Copy link
Contributor

Thanks for the fix, can you rebase against master and sign the CLA?

@danielnelson danielnelson changed the base branch from release-1.3 to master May 25, 2017 20:19
@danielnelson
Copy link
Contributor

I change the base branch on github, but unfortunately it still needs rebased to remove all the commits in the branch that are not in master.

@danielnelson danielnelson added regression something that used to work, but is now broken bug unexpected problem or unintended behavior labels May 25, 2017
@snardone
Copy link
Contributor Author

Hi Daniel, I signed the NDA. I apologize for the delay, my git skills are still a work-in-progress, I hope I did this correctly. If not please let me know.

@danielnelson
Copy link
Contributor

Can you do an interactive rebase and remove the commits that were added when I switched the base branch? It goes something like this (assuming the influxdata/telegraf repos is a remote called origin and your forks remote is called fork):

git checkout fix_mongo_input_panic
git rebase -i origin/master
... remove commits that aren't yours in the editor that pops up ...
git push fork fix_mongo_input_panic

@snardone
Copy link
Contributor Author

Daniel, I'm not sure how to resolve this:

$ git status
On branch fix_mongo_input_panic
Your branch is up-to-date with 'origin/fix_mongo_input_panic'.
nothing to commit, working tree clean
$ git rebase -i telegraf/master

Editor pops up, set all but af90b0a to 'r'.

      1 pick af90b0a9 Check for nil on mongo input, fix for panic.
      2 r 619d4d5d Use go 1.8.1 for CI and Release builds (#2732)
      3 r 547be87d Clarify retention policy option for influxdb output
      4 r bc474d3a Clarify retention policy option for influxdb output
      5 r 20ab8fb2 Update telegraf.conf
      6 r 499495f8 Don't log error creating database on connect (#2740)
      7 r b0a2e8e1 Add initial documentation for rabbitmq input. (#2745)
      8 r ce203dc6 fix close on closed socket_writer (#2748)
      9 r 5e70cb3e Improve redis input documentation (#2708)
     10 r 18fd2d98 Return an error if no valid patterns. (#2753)
     11 r ebef47f5 fix systemd path in order to add compatibility with SuSe (#2499)
     12 r ff704fbe Add SLES11 support to rpm package (#2768)
     13 r feaf7691 Add missing plugins to README
     14 r b16eb6ea split metrics based on UDPPayload size (#2795)
     15 r 27b89dff Only split metrics if there is an udp output (#2799)
     16 r 99b53c87 Add back the changelog entry for 2141
     17 r 2bc5594b Add release date for 1.3.0
     18 r 1a7cc9f4 Check for nil on mongo input, fix for panic.

I get this conflict:

git status
On branch fix_mongo_input_panic
Your branch is up-to-date with 'origin/fix_mongo_input_panic'.
nothing to commit, working tree clean

Otherwise, please use 'git reset'
interactive rebase in progress; onto 7d7206b3
Last commands done (2 commands done):
   pick af90b0a9 Check for nil on mongo input, fix for panic.
   r 619d4d5d Use go 1.8.1 for CI and Release builds (#2732)
Next commands to do (16 remaining commands):
   r 547be87d Clarify retention policy option for influxdb output
   r bc474d3a Clarify retention policy option for influxdb output
You are currently rebasing branch 'fix_mongo_input_panic' on '7d7206b3'.

nothing to commit, working tree clean
Could not apply 619d4d5d29cddc54e83fdb58f9c0a68961562905... Use go 1.8.1 for CI and Release builds (#2732)

In my setup:

$ git remote -v
origin	https://github.com/snardone/telegraf (fetch)
origin	https://github.com/snardone/telegraf (push)
telegraf	https://github.com/influxdata/telegraf (fetch)
telegraf	https://github.com/influxdata/telegraf (push)

Any ideas? Definitely going to spend some time on improving my git skills this weekend.

@snardone
Copy link
Contributor Author

Ignore that, using 'd' instead of 'r', misread the instructions.

@snardone
Copy link
Contributor Author

$ git push origin fix_mongo_input_panic
Counting objects: 5, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (5/5), done.
Writing objects: 100% (5/5), 595 bytes | 0 bytes/s, done.
Total 5 (delta 4), reused 0 (delta 0)
remote: Resolving deltas: 100% (4/4), completed with 3 local objects.
To https://github.com/snardone/telegraf
   803fad10..5e1bc727  fix_mongo_input_panic -> fix_mongo_input_panic

I'll get the hang of this!

@danielnelson
Copy link
Contributor

Based on the output of git remote, you need to:

Update your master branch with the latests from the influxdata/telegraf repo

git checkout master
git pull --ff-only telegraf master

Interactively rebase your branch onto master

git checkout fix_mongo_input_panic
git rebase -i master

At this point you have the list of commits. The only new commit you have is bf8349f, so you can remove the other lines so that your editor looks just has a single pick:

pick bf8349f1 Check for nil on mongo input, fix for panic.

When you run git log telegraf/master... it will show only your one commit. Now you will need to force push to your remote branch.

git push origin fix_mongo_input_panic -f

@snardone
Copy link
Contributor Author

Daniel, thanks for your help, I've followed your instructions, let me know if you need anything else.

@danielnelson danielnelson merged commit e7f9db2 into influxdata:master May 30, 2017
danielnelson pushed a commit that referenced this pull request May 30, 2017
(cherry picked from commit e7f9db2)
@danielnelson
Copy link
Contributor

Thanks, this will be in the 1.3.1 release.

jeichorn pushed a commit to jeichorn/telegraf that referenced this pull request Jul 24, 2017
@snardone snardone deleted the fix_mongo_input_panic branch January 9, 2018 00:13
maxunt pushed a commit that referenced this pull request Jun 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug unexpected problem or unintended behavior regression something that used to work, but is now broken
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants