<div dir="ltr"><div dir="ltr"><div>Hello,</div><div><br></div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">Le mer. 11 mai 2022 à 00:20, melanie witt <<a href="mailto:melwittt@gmail.com">melwittt@gmail.com</a>> a écrit :<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Tue May 10 2022 10:44:32 GMT-0700 (Pacific Daylight Time), Ghanshyam <br>
Mann <<a href="mailto:gmann@ghanshyammann.com" target="_blank">gmann@ghanshyammann.com</a>> wrote:<br>
>   ---- On Tue, 10 May 2022 12:24:08 -0500 Stephen Finucane <<a href="mailto:stephenfin@redhat.com" target="_blank">stephenfin@redhat.com</a>> wrote ----<br>
>   > On Tue, 2022-05-10 at 18:03 +0100, Stephen Finucane wrote:<br>
>   > > On Tue, 2022-05-10 at 09:56 -0500, Matthew Thode wrote:<br>
>   > > > Hi all,<br>
>   > > ><br>
>   > > > It looks like the latest oslo.log update is failing to pass tempest.<br>
>   > > > If anyone is around to look I'd appreciate it.<br>
>   > > > <a href="https://review.opendev.org/840630" rel="noreferrer" target="_blank">https://review.opendev.org/840630</a><br>
>   > ><br>
>   > > Repeating from the review, this seems to be caused by [1]. I'm not entirely sure<br>
>   > > why Tempest specifically is unhappy with this. I'll try to find time to look at<br>
>   > > this. Hopefully Takashi-san will beat me to it though, as the author of that<br>
>   > > change ;)<br>
>   ><br>
>   > Actually, maybe not that hard to solve. I think [1] will fix the issue. Tempest<br>
>   > doesn't use keystoneauth1 like everyone else so they didn't get the global<br>
>   > request ID stuff for free. We need to pass some additional context when logging<br>
>   > to keep this new version of oslo.log happy.<br>
>   ><br>
>   > If you've thoughts on this, please let me know on the review. The other option<br>
>   > is to revert the change on oslo.log and cut a new release, blacklisting this<br>
>   > one.<br>
> <br>
> I am ok with tempest change but IMO oslo.log changes are backward-incompatible with not<br>
> much value? may be it should check if there is global request-id then of otherwise skip?<br></blockquote><div><br></div><div>I agree with that, we need to better handle this point in oslo.log.</div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
+1, it feels like this is a bug in oslo.log. Prior to the <br>
global_request_id change, it did the following test before using the <br>
logging_context_format_string [2]:<br>
<br>
         if record.__dict__.get('request_id'):<br>
             fmt = self.conf.logging_context_format_string<br>
         else:<br>
             fmt = self.conf.logging_default_format_string <br></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
which protected against a KeyError in the case of request_id. It seems <br>
like something similar needs to be added for global_request_id.<br></blockquote><div><br></div><div>If we apply the same logic for `global_request_id` then we will open the door of an implicit remplacement of the used format. By example if `request_id` is given and not `global_request_id` then we will override the context format with the default format implicitly.</div><div><br></div><div>I'd rather suggest to init `global_request` with a default value (`None`) as we did previously with other context var format => <a href="https://opendev.org/openstack/oslo.log/commit/c50d3d633ba">https://opendev.org/openstack/oslo.log/commit/c50d3d633ba</a></div><div><br> </div><div>Thoughts?<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
-melanie<br>
<br>
[2] <br>
<a href="https://github.com/openstack/oslo.log/blob/ebdee7f39920ad5b4268ee296952432b0d41a375/oslo_log/formatters.py#L468" rel="noreferrer" target="_blank">https://github.com/openstack/oslo.log/blob/ebdee7f39920ad5b4268ee296952432b0d41a375/oslo_log/formatters.py#L468</a><br>
<br>
>   > [1] <a href="https://review.opendev.org/c/openstack/tempest/+/841310" rel="noreferrer" target="_blank">https://review.opendev.org/c/openstack/tempest/+/841310</a><br>
>   ><br>
>   > ><br>
>   > > Stephen<br>
>   > ><br>
>   > > [1] <a href="https://review.opendev.org/c/openstack/oslo.log/+/838190" rel="noreferrer" target="_blank">https://review.opendev.org/c/openstack/oslo.log/+/838190</a><br>
>   ><br>
>   ><br>
>   ><br>
> <br>
<br>
<br>
</blockquote></div><br clear="all"><br>-- <br><div dir="ltr" class="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr"><div>Hervé Beraud</div><div>Senior Software Engineer at Red Hat</div><div>irc: hberaud</div><div><a href="https://github.com/4383/" target="_blank">https://github.com/4383/</a></div><div><a href="https://twitter.com/4383hberaud" target="_blank">https://twitter.com/4383hberaud</a><br><br></div></div></div></div></div></div></div></div></div></div></div></div></div></div>