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

exec plugin: log no such dataset registered error at INFO level #1327

Closed
wants to merge 1 commit into from

Conversation

adamvduke
Copy link

Spent a few hours debugging and ended up having to recompile on a production server with debug enabled to get an error message about a configuration mistake. It seems like if you are sending metrics with unregistered types that should be readily available at the INFO level.

@mfournier
Copy link

Sorry about that @adamvduke :-(

What was the actual configuration mistake which led to this error message (not) getting printed out ?

@adamvduke
Copy link
Author

Hey @mfournier,

The intention wasn't to cause a :-(

I've actually been really happy with the project in general, although googling for things can be hard at times. 🍻

We are using the exec plugin to run a custom script that collects some metrics from postgres at a much longer interval than we report other postgres metrics. The line in the script that caused the problem was something like the following.

echo "PUTVAL $HOSTNAME/okr/project-name/gauge-active_count interval=$INTERVAL N:$active_count"

We use graphite as our metrics store and I was trying to namespace the metric underneath 'okr' with the project's name. Once I found the error message with debug logging, I knew exactly what the problem was because I had run up against this problem in the past. We ended up just removing the namespacing and everything worked as expected. The fix was to use the following line:

echo "PUTVAL $HOSTNAME/okr/gauge-project_name_active_count interval=$INTERVAL N:$active_count"

@mfournier
Copy link

Ok I see, thanks for the clarifications !
From what I understand, the following message would have got printed out somewhere: -1 Type``%s' isn't defined. Not really sure where that ends up in the case of the exec plugin though.

So IMHO the right fix would be to improve badly formatted identifiers interception. The fact this specific code path was taken indicates the identifier successfully passed through the parse_identifier() function despite it not complying with the naming scheme.

Let's leave this open for the record, and see if one of us or someone else comes up with a better solution.

@mfournier mfournier added the Bug A genuine bug label Oct 30, 2015
@octo
Copy link
Member

octo commented Dec 1, 2015

Thanks for reporting this, @adamvduke, and sorry for the debugging work required to track this down! This is a DEBUG message because it is rare, but when it occurs it's incredibly spammy.

The best place to tackel this would be src/utils_cmd_putval.c. Right now the exec plugin passes in stdout as a FILE*, which is hooked up to /dev/null (unless running in the foreground), so the error messages are lost. What we could do instead is pass in NULL and use the ERROR() in this case.

P.S.: Every once in a while I search for tweets mentioning collectd – that's annoyingly noisy ;-)

@octo octo changed the title log no such dataset registered error at INFO level exec plugin: log no such dataset registered error at INFO level Dec 2, 2015
@mfournier mfournier modified the milestone: 5.4 Jan 21, 2016
@adamvduke
Copy link
Author

adamvduke commented Oct 25, 2016

I was looking at old PR's and noticed this is labeled as 'pending contributor action'. I'm assuming that is me? I'm not very confident in my familiarity with the codebase or my c foo, so I'm not sure I'll be able to make the suggested changes. It also looks like a lot of things have likely changed since the issue was originally opened. It seems like the best course of action may be to close the issue, but leave it around for documentation's sake?

Sorry, I clicked the close button by mistake.

@adamvduke adamvduke closed this Oct 25, 2016
@octo octo added Fix A pull request fixing a bug and removed Bug A genuine bug labels Jan 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fix A pull request fixing a bug Pending contributor action
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants