[
https://issues.apache.org/jira/browse/BEAM-6518?focusedWorklogId=191205&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-191205
]
ASF GitHub Bot logged work on BEAM-6518:
----------------------------------------
Author: ASF GitHub Bot
Created on: 28/Jan/19 19:09
Start Date: 28/Jan/19 19:09
Worklog Time Spent: 10m
Work Description: ryan-williams commented on issue #7638: [BEAM-6518] fix
python metric-querying bug
URL: https://github.com/apache/beam/pull/7638#issuecomment-458261266
[This test I
added](https://github.com/apache/beam/pull/7638/files#diff-520d5662bc2c53338f2f2b497e9b14e1R79)
shows an example of a step-name where the old code would have a false negative
(i.e. would omit a metric in response to a query that should have returned it);
paraphrasing:
```
name = MetricName('ns1', 'name1')
key = MetricKey('Step10/Step1', name)
filter = MetricsFilter().with_step('Step1')
MetricResults.matches(filter, key) # previously False, now True (which is
correct/intended)
```
This previously would have failed because:
- the `Step1` in `Step10` would be found as a potential match for `Step1`
- it would be ruled out due to not being an exact match
- algorithm would incorrectly `return False`, missing the actual `Step1`
which comes later
Most likely no one has ever hit this in production. However, to me it is a
clear-cut bug-fix. `MetricResults._matches_sub_path` is not supposed to have
this edge-case.
In fact, talking it through here, it was unnecessarily complicated as a
result of being based on `str.find()` and then retroactively checking against
`/`-delimiters. I've rewritten it to more directly do what it is supposed to:
split on `/`s and see if the "filter" is a sub-list of the "actual"
`MetricKey`'s "step".
I also removed some redundant lines (re-assigning the same values to the
same vars) from the tests.
lmk what you think!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
Issue Time Tracking
-------------------
Worklog Id: (was: 191205)
Time Spent: 40m (was: 0.5h)
> Substring-matching bug in Python metrics filtering
> --------------------------------------------------
>
> Key: BEAM-6518
> URL: https://issues.apache.org/jira/browse/BEAM-6518
> Project: Beam
> Issue Type: Bug
> Components: sdk-py-core
> Affects Versions: 2.9.0
> Reporter: Ryan Williams
> Assignee: Ryan Williams
> Priority: Trivial
> Time Spent: 40m
> Remaining Estimate: 0h
>
> {{[MetricResults._matches_sub_path|https://github.com/apache/beam/blob/5b46b02b49ca1c5c18682427a5a4a25596ca4287/sdks/python/apache_beam/metrics/metric.py#L152-L163]}}
> can be fooled into a false-negative if a path component before the one it is
> looking for is a superstring of the one it's looking for.
> It's not particularly likely to happen, but worth fixing anyway.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
