Page MenuHomePhabricator

Set a timeout for regex parsing in the Eventlogging processors
Open, Needs TriagePublic


As a follow up for T200630, the best and most flexible solution seems to be to add a maximum execution time limit while parsing (applying various regexes) to the UA string.

An initial investigation from @Milimetric looks promising:

Event Timeline

elukey triaged this task as High priority.Jul 31 2018, 6:15 AM
elukey created this task.

Using a contextmanager decorator and the built-in TimeoutError exception (if Py3 only) the code could be much simpler, something like:

import signal

from contextlib import contextmanager

def timeout_handler(signum, frame):
    raise TimeoutError

def timeout(seconds):
    prev_handler = signal.signal(signal.SIGALRM, timeout_handler)  # Store the previous signal handler
    signal.alarm(seconds)  # Set the timeout

        yield  # Yield to the context

        signal.alarm(0)  # Reset the alarm
        signal.signal(signal.SIGALRM, prev_handler)  # Restore the previous handler

### Usage
    with timeout(3):
        # call the ua parser
except TimeoutError:
    # handle the failed case

Thanks @Volans, didn't know about the contextmanager, that's nicer. And @elukey, as for concerns about the use of signal complicating the code, after reading more about it I think it's fine. Signals are received on the main thread as you said in the email, and the kafka-python threads are started in another part of the code. So all's good.

elukey removed elukey as the assignee of this task.Aug 24 2018, 1:46 PM
fdans moved this task from Incoming to Operational Excellence on the Analytics board.
Milimetric closed this task as Declined.Mar 4 2019, 4:52 PM

Closing because we haven't seen the issue resurface. Our temporary fix seems fine until we transition off of EventLogging, hopefully sooner than later.

Nuria added a comment.Mar 5 2019, 8:50 PM

Rather than closing I am going to move this one to "deprioritized" for now so as not to loose track of it completely.

Nuria reopened this task as Open.Mar 5 2019, 8:50 PM
Nuria lowered the priority of this task from High to Low.
Nuria raised the priority of this task from Low to Needs Triage.
Nuria moved this task from Operational Excellence to Deprioritized on the Analytics board.