Add module display name to module related server events

[mulby]

This information will likely be used frequently for analytics purposes, so we would like to denormalize here to avoid having to join with the modulestore later.

We would like this feature to make it in to the release tomorrow.

A few notes:

• Wasn't sure where the best place to put this logic is. Very open to suggestions.
• I figured the best way to communicate what we are trying to do was to submit a diff. I'm not attached to any of this.
• Once the approach is approved, I'll nail it down with some tests etc.

Fixes: AN-594
Reviewers: @cpennington, @rocha, @brianhw

[mulby]

Not sure where to put this exactly. I need to wrap the execution of the module logic from a location where I have easy access to the "display_name_with_default" information. I'm fine moving this anywhere really.

[brianhw]

Seems right to me.

[rocha]

What about if the context is only for instance.handle?

with tracker.context(...):
  resp = instance.handle(handler, req, suffix)

[mulby]

Yup, I like that better.

[cpennington]

Why is the context named with the id of the block? It seems like both the id and the display_name should be part of a context named something like 'xblock'. Although, maybe that's just me not knowing how your contexts work?

[cpennington]

Oh... it's to handle nesting, isn't it.

[cpennington]

Also, it's worth noting that this doesn't wrap any of the view execution (rendering student_view). This is only wrapping xblock handlers.

[mulby]

Yes it's to handle nesting. I'm not sure if this function can be called multiple times in the same request, but if it can, say on parent and child xblocks, then I'd rather not overwrite the parent context when this is called for the child. If they have different names, then it is smart enough to "push" the child context on top of the stack and it's keys will override the parent context, however, when the child context is exited, the parent values will still be there.

If, however, this function is only called once per request, then it should be safe to use the same name.

At a minimum I should probably put in a comment to this effect since it's non-obvious.

[mulby]

Is there a better place to wrap that would ensure the context was populated for both views and handlers?

[brianhw]

LGTM. 👍

[mulby]

@cpennington @rocha - let me know if this is good to merge, we would like to get it in to the release tomorrow.

[rocha]

🚀

[mulby]

Rebased and squashed on to the tip of master. @cpennington - we are planning on cherry picking this commit on to the release branch, but don't want to do so without your approval. Let me know what you think.

[mulby]

@cpennington we decided not to ship this with the release, so the pressure is off for the time being.

[mulby]

Updated to use a static context name

[cpennington]

👍