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

cmd/scollector: fix empty host= tags not working after PR #1856 #1871

Merged
merged 1 commit into from
Aug 16, 2016

Conversation

gbrayut
Copy link
Contributor

@gbrayut gbrayut commented Aug 16, 2016

The add function in cmd/scollector/collectors/collectors.go uses host= to indicate that tag should be omitted

See

// automatically added. If the value of the host key is the empty string, it

This should fix the vsphere errors on ny-bosun01:

error: collectors.go:256: Invalid tagset discovered: {disk=CO-EQL,host=}. Skipping datapoint. Added from: vsphere.go:138
error: collectors.go:256: Invalid tagset discovered: {disk=datastore1,host=}. Skipping datapoint. Added from: vsphere.go:105
error: collectors.go:256: Invalid tagset discovered: {disk=datastore1,host=}. Skipping datapoint. Added from: vsphere.go:136
error: collectors.go:256: Invalid tagset discovered: {disk=datastore1,host=}. Skipping datapoint. Added from: vsphere.go:138

@captncraig
Copy link
Contributor

My only worry is that the invariant for Clean is now violated. Clean should always result in a valid datapoint (can be sent and accepted by opentsdb as is), or an error. A solution more specific to AddTS might be to move the host fixing up above the call to Clean.

@gbrayut
Copy link
Contributor Author

gbrayut commented Aug 16, 2016

Ah... I couldn't find that (bit scatter brained). Let me try that instead.

@gbrayut
Copy link
Contributor Author

gbrayut commented Aug 16, 2016

Yep... that works and is much better. I'll let this run on ny-bosun01 overnight as well but don't expect any issues.

Actually I'm going to merge this now since it is a trivial change of order of operations and it make testing the other branch easier.

@gbrayut gbrayut merged commit e0246a9 into master Aug 16, 2016
@gbrayut gbrayut deleted the emptyhost branch August 16, 2016 03:16
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.

2 participants