CloudWatch EMF Route RequestLifecycleHooks

[derek-globus]

What?

1. Added support for RequestLifecycleHook registration to the flask ActionProviderBlueprint class.
   Classes may be provided at Blueprint instantiation time to register before, after, or
   teardown functionality to wrap all view invocation.

2. Added a CloudWatchEMFLogger RequestLifecycleHook class.
   When attached to an ActionProviderBlueprint, it will emit request count, latency, and
   response category (2xxs, 4xxs, 5xxs) count metrics through CloudWatch EMF. Metrics
   are emitted both at the aggregate AP and at the individual route level.

Dimension Sets

• {ActionProvider: <action-provider-name>}
• {ActionProvider: <action-provider-name>, Route: <route-type>}
  • <route-type> in this case is provided by the library not the user view func, so it'll be "run", "release", "resume", etc.

Metrics:

• Count - Count of invocations against an AP or route
• 2XXs - Count of invocations against an AP or route resulting in a 2XX
• 4XXs - Count of invocations against an AP or route resulting in a 4XX
• 5XXs - Count of invocations against an AP or route resulting in a 5XX
• RequestLatency - Duration of invocation handling for an AP or route (in milliseconds)

Why?

Provide an easy mechanism for Action Providers to get usage metrics for both their aggregate Action Provider & specific routes of action providers

Usage

from globus_action_provider_tools.flask import ActionProviderBlueprint
from globus_action_provider_tools.flask.request_lifecycle_hooks import (
    CloudWatchMetricEMFLogger,
)

aptb = ActionProviderBlueprint(
    name="TrackedActionProvider",
    import_name=__name__,
    url_prefix="/tracked",
    provider_description=ap_description,
    request_lifecycle_hooks=[
        CloudWatchMetricEMFLogger(namespace="ActionProviders", action_provider_name="TrackedActionProvider")
    ],
)

Testing

unit tested

[sirosen]

I know I'm jumping into a naming debate here, but I'd like us to avoid using "middleware" for this, specifically because Flask and Werkzeug already have a type of object they support called a "middleware".
In their case, it's something which wraps a whole app, e.g.

app = app_factory()
app = CORSMiddleware(app, cors_config)

So I'm trying to find an alternative name which means basically the same thing.
"Lifecycle Hooks" or something like that?
Observer? (Not quite right, the hooks can modify data...)
HookCollection?

I'm struggling for a good name, but want to avoid an already-taken framework term.

[derek-globus]

I know I'm jumping into a naming debate here, but I'd like us to avoid using "middleware" for this, specifically because Flask and Werkzeug already have a type of object they support called a "middleware".

Yeah I didn't expect that I'd get this one through undebated 😬.

My reasoning for sticking with Middleware was that while flask does reference the term middleware, their "support" for it isn't really support. It's "hey wrap our app with a class that can do stuff and make sure it calls the app as well; we'll conventionally call that thing Middleware!" (ref)

I believe the interface I've exposed (a class that taps into their builtin before_request, after_request, teardown_request hooks) actually fits more precisely with the term middleware.

So I'm trying to find an alternative name which means basically the same thing.
"Lifecycle Hooks" or something like that?
Observer? (Not quite right, the hooks can modify data...)
HookCollection?

RequestLifecycleHooks or APRequestLifeCycleHooks would probably be the only one that makes sense in that vein. Middleware is a more accurate naming, but to your point it may make sense to avoid any hint of conflict w/ Flask.

[derek-globus]

I renamed these functions because they were named with test_ which caused pytest to think they were tests to be run when in fact they are utility callables for other tests to supply to the action provider blueprint as action provider view functions.

Example failure: https://github.com/globus/action-provider-tools/actions/runs/6510610219/job/17684687247?pr=105

[ada-globus]

Agree with all of the above—"middleware" feels like the clearest, but the potential for confusion is high in this context. I like RequestLifecycleHooks quite a bit.

[derek-globus]

I ran this PR by Stephen Zimmerman & he had the same comment about Middlware as terminology but was content with RequestLifecycleHooks.

Updating in new rev

[sirosen]

My reasoning for sticking with Middleware was that while flask does reference the term middleware, their "support" for it isn't really support. It's "hey wrap our app with a class that can do stuff and make sure it calls the app as well; we'll conventionally call that thing Middleware!" (ref)

I believe the interface I've exposed (a class that taps into their builtin before_request, after_request, teardown_request hooks) actually fits more precisely with the term middleware.

I agree with you, but I think the potential for confusion is real enough (e.g. me in 6 months) that we should use a slightly less optimal name because the best name was (rudely!) taken from us.

I'm on the RequestLifecycleHooks bandwagon now.

I'll circle back to do a real content-ful review later, but I think it's all good from a quick scan.
I scoffed at the fmt: off for a tick until I saw what it was wrapping, and felt like "okay, yeah, maybe we don't need to black-format everything". 😉 I'm sure you had a similar judgement which led you to turning it off.

[derek-globus]

I scoffed at the fmt: off for a tick until I saw what it was wrapping, and felt like "okay, yeah, maybe we don't need to black-format everything". 😉 I'm sure you had a similar judgement which led you to turning it off.

cough cough flows-logging cough cough

[derek-globus]

For some reason on py3.12 windows (exclusively) this request latency comes back as 0.0 (my initial condition was request_latency > 0).

I don't know why that was the case (maybe it's inheriting freeze_time from other tests) but felt comfortable loosening the test to just verify that this is a number rather than a specific number/number range.

Example failure: https://github.com/globus/action-provider-tools/actions/runs/6510955778/job/17685743540

[ada-globus]

Still reviewing, but I have a comment that might be relevant to this, so I will push that before finishing review.

[ada-globus]

Still reviewing, but I have a comment that might be relevant to this, so I will push that before finishing review.

[ada-globus]

Looks great! Approving now as I don't have any substantive concerns, but have a couple minor notes for consideration.