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.

Sign up for GitHub

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

Jump to bottom

User may overwrite observations with special symbols #435

Open
DifferentialOrange opened this issue Feb 6, 2023 · 1 comment
Open

User may overwrite observations with special symbols #435

DifferentialOrange opened this issue Feb 6, 2023 · 1 comment
Assignees
@DifferentialOrange
Labels
bug Something isn't working

Comments

@DifferentialOrange
Copy link
Member

Due to the implementation of internal storage, it is possible to overwrite existing observations if they have two or more labels:

tarantool> c = metrics.gauge('coll')
---
...

tarantool> c:set(3, {lab = 'val', lab2 = 'val2'})
---
...

tarantool> c:collect()
---
- - label_pairs:
      lab2: val2
      lab: val
    timestamp: 1675671598982881
    value: 3
    metric_name: coll
...

tarantool> c:set(4, {['lab\tval\tlab2'] = 'val2'})
---
...

tarantool> c:collect()
---
- - label_pairs:
      "lab\tval\tlab2": val2
    timestamp: 1675671636647270
    value: 4
    metric_name: coll
...

It is highly unlikely that someone would become a victim of this bug unintentionally, but this behavior still seems a bit problematic.

Moreover, using special symbols breaks prometheus export:

# HELP coll 
# TYPE coll gauge
coll{lab	val	lab2="val2"} 4

error while linting: text format parsing error in line 3: expected '=' after label name, found 'v'
@DifferentialOrange DifferentialOrange added bug Something isn't working teamE labels Feb 6, 2023
@DifferentialOrange DifferentialOrange self-assigned this Feb 6, 2023
DifferentialOrange added a commit that referenced this issue Feb 15, 2023
@DifferentialOrange
api: deprecate control characters
c50b77f 
Deprecate using control characters (including tabulation) in collector
kind, name, observation and global labels. Using such symbols may result
in registry collision (some observations may be lost), as well as
corrupted plugin output.

Part of #435, part of tarantool/tarantool#7725
DifferentialOrange added a commit that referenced this issue Feb 16, 2023
@DifferentialOrange
api: deprecate control characters
2c95562 
Deprecate using control characters (including tabulation) in collector
kind, name, observation and global labels. Using such symbols may result
in registry collision (some observations may be lost), as well as
corrupted plugin output.

Part of #435, part of tarantool/tarantool#7725
@filonenko-mikhail
Copy link
Contributor

filonenko-mikhail commented Apr 13, 2023 •
edited

The metric counter is code hot path. Be careful it seems that new if can decrease perf a little.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@DifferentialOrange DifferentialOrange

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@filonenko-mikhail @DifferentialOrange @TarantoolBot