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 order of checking if value is bool #428

Merged
merged 2 commits into from
Sep 17, 2020

Conversation

bjozet
Copy link
Contributor

@bjozet bjozet commented Aug 27, 2019

bool is subclass of int and will match against a check in
utils.is_numeric(). This commit checks if v is bool before anything
else, so we catch it early.

>>> int.__subclasses__()
[<type 'bool'>]
>>> isinstance(True, (int))
True
>>> isinstance(True, (bool))
True

Previous behaviour was that jolokia tried to send True for booleans as
a value to opentsdb, which isn't allowed, and caused
java.lang.NumberFormatException: For input string: "True" in
application logs.

bjozet added 2 commits August 27, 2019 12:54
bool is subclass of int and will match against a check in
`utils.is_numeric()`. This commit checks if `v` is bool before anything
else, so we catch it early.

```
>>> int.__subclasses__()
[<type 'bool'>]
>>> isinstance(True, (int))
True
>>> isinstance(True, (bool))
True
```

Previous behaviour was that jolokia tried to send `True` for booleans as
a value to opentsdb, which isn't allowed, and caused
`java.lang.NumberFormatException: For input string: "True"` in
application logs.
jolokia has some metrics with single-tics in them as such:

`CodeHeap 'non-profiled nmethods'":
{"init":2555904,"committed":18087936,"max":121737216,"used":14709504}`

This would translate into invalid metric-names. Opentsdb accepts this
regexp in metric/tags: `[-_./a-zA-Z0-9]+`

Also changed the single-line .replace().replace() string operation into
separate lines for readability.
@bjozet
Copy link
Contributor Author

bjozet commented Aug 30, 2019

Piggy-backing another fix into this PR to fix invalid metric names generated from jolokia. No use in having separate PRs as this one hasn't been reviewed yet anyway.

@johann8384 johann8384 merged commit de6d2a3 into OpenTSDB:master Sep 17, 2020
@johann8384 johann8384 added this to the 1.3.3 milestone Sep 17, 2020
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