Merge lp://staging/~mbp/launchpad/666765-features-no-reasons into lp://staging/launchpad

Proposed by Martin Pool
Status: Merged
Approved by: Martin Pool
Approved revision: no longer in the source branch.
Merged at revision: 14512
Proposed branch: lp://staging/~mbp/launchpad/666765-features-no-reasons
Merge into: lp://staging/launchpad
Diff against target: 193 lines (+86/-6)
6 files modified
lib/canonical/launchpad/webapp/errorlog.py (+36/-3)
lib/lp/app/browser/tests/base-layout.txt (+0/-1)
lib/lp/app/templates/base-layout.pt (+0/-1)
lib/lp/services/features/flags.py (+15/-0)
lib/lp/services/features/tests/test_webapp.py (+29/-1)
utilities/bsondump (+6/-0)
To merge this branch: bzr merge lp://staging/~mbp/launchpad/666765-features-no-reasons
Reviewer Review Type Date Requested Status
j.c.sackett (community) Approve
Review via email: mp+84050@code.staging.launchpad.net

Commit message

[r=jcsackett][bug=666765] record features and scopes in the timeline; don't show scopes in the page footer

Description of the change

Per <https://bugs.launchpad.net/launchpad/+bug/666765> users may be subjected to or excluded from particular features without being allowed to know why.

We could possibly track the features and scopes in the timeline (if it's not already) to make it easy for developers to get at it.

To post a comment you must log in.
Revision history for this message
j.c.sackett (jcsackett) wrote :

I'm not sure I love just killing this without having the alternative debug methods available. How much work is it to get this into the timeline as part of this branch?

review: Needs Information
Revision history for this message
Martin Pool (mbp) wrote :

On 2 December 2011 05:13, j.c.sackett <email address hidden> wrote:
> Review: Needs Information
>
> I'm not sure I love just killing this without having the alternative debug methods available. How much work is it to get this into the timeline as part of this branch?

probably not too much. i will have a go. that would be useful for
oopses etc anyhow.

Am I correct in thinking the timeline data is treated with sufficient
care that it's ok to put sensitive data in there? Untrusted users
never see it?

--
Martin

Revision history for this message
Robert Collins (lifeless) wrote :

On Wed, Dec 7, 2011 at 6:13 PM, Martin Pool <email address hidden> wrote:
> On 2 December 2011 05:13, j.c.sackett <email address hidden> wrote:
>> Review: Needs Information
>>
>> I'm not sure I love just killing this without having the alternative debug methods available. How much work is it to get this into the timeline as part of this branch?
>
> probably not too much.  i will have a go.  that would be useful for
> oopses etc anyhow.
>
> Am I correct in thinking the timeline data is treated with sufficient
> care that it's ok to put sensitive data in there?  Untrusted users
> never see it?

All Canonical staff can see the timelines; its fine to put nonpublic
stuff in there, its not fine to put security sensitive things in
there.

However, I don't think you need the timeline for this, just some extra messages.

-Rob

Revision history for this message
Martin Pool (mbp) wrote :

just debug-level log messages?

Revision history for this message
Robert Collins (lifeless) wrote :

On Wed, Dec 7, 2011 at 6:35 PM, Martin Pool <email address hidden> wrote:
> just debug-level log messages?

extra oops messages - they will be collated into the oops and shown in
the variables summary at the top (rather than folk having to trawl
through to see what feature scopes were evaluated.

Revision history for this message
Martin Pool (mbp) wrote :

ok, so that was a bit more complicated, because the messages are accumulated process wide not per request.

per a later suggestion on irc, this now pulls them out of the request when the oops fires.

Revision history for this message
j.c.sackett (jcsackett) wrote :

Thanks for doing the extra work there, Martin. This looks good to me.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.