Comment 1 for bug 1407649

Revision history for this message
Zhi Yan Liu (lzy-dev) wrote :

Thanks Stuart, I think this is a good point to discuss.

>>> The initialisation/configurization of osprofiler is different to other pipeline entries, such as keystone.

I think a key reason is middleware is only a min optional part of osprofiler, for example in proposed change of zarqa [0], it didn't use paste pipeline, but it still need "osprofiler.notifier.create()" call due to whatever we use web middleware of osprofiler, we all need to initialize notifier for osprofiler internal usage base on it integrated project, for example, in glance we could use oslo.messaging notifier directly due to glance integrated oslo.messaging, but for trove, currently oslo.messaging integration change is still wip (very close [2]) so we need to write a customized notifier in project, and create and set notifier to osprofiler. So overall, I mean osprofiler can't bootstraps itself without extra initialization step.

Another reason of why osprofiler way unlikes "there is no explicit call to initialize the keystone middleware" is that historically as the first osprofiler integrated project glance team asked to add a main 'enabled' option to switch osprofiler if disabled totally (and I think it's necessary/useful, especially for the concern from performance perspective).

>>> the keystone middleware options are defined in the middleware itself

I trend to agree this, we can move there essential options from integrated project to a proper place in osprofiler, e.g. 'enabled', 'hmac_keys', and 'trace_sqlalchemy', however project probably need to add own options for particular profiling requirement, for example, in zaqar we'd like to allow operator to configure profile switcher on different driver [3].

So in sort, I think the key to me is that, osprofiler is not just middleware, but it's a lib. So we can't just 'to remove completely the "if cfg.CONF.profiler.enabled" code block once the [osprofiler] stanza and the relevant options is available.' then to make it works.

[0] Change I32565de6c447cd5e95a0ef54a9fbd4e571c2d820
[1] Change I580cce8d2b3c4ec9ce625ac09de6f14e1249f6f5
[2] Change Ibd886f3cb4a45250c7c434b3af711abee266671c
[3] https://review.openstack.org/#/c/141356/6/zaqar/profile.py #L33-42

If I miss anything please feel free to point it out.