feat: deprecate track_function and publish in ModuleSystem [BD-13]

[Agrendalath]

Description

As a part of the ModuleSystem removal, we want to get rid of its remaining attributes:

• track_function,
• publish.

Supporting information

Sandbox instance: https://pr30046.sandbox.opencraft.hosting/
Use staff@example.com:edx to log in.

Testing instructions

1. We have replaced the remaining usages of track_function with publish, so it makes sense to check these two together.
2. Log into the sandbox instance (private link) and check tracking logs for the next actions.
3. Check that the grading is working. You can use this XBlock in the LMS and Preview. Check that the Studio view is working correctly too.
4. Check that the "Show answer" button emits events.
5. Check that the completion events are working (the completion is already enabled on the sandbox).

Other information

Private-ref: BB-5518

[openedx-webhooks]

Thanks for the pull request, @Agrendalath!

When this pull request is ready, tag your edX technical lead.

[natabene]

@Agrendalath Just checking the status, as this has been open as a draft for a while.

[natabene]

Author will pick this up soon.

[Agrendalath]

@ormsbee, would you like to do a sanity check of this PR?

[pkulkark]

@Agrendalath I tried to test this out on the sandbox instance. But I got an error in the chemical equations problem:

[Agrendalath]

@pkulkark, it was not related to the PR. It's an old sandbox, so I manually deleted all keys matching default:1:safe_exec.* from the cache. safe_exec is working correctly now.

[ormsbee]

@Agrendalath: Looking through this now...

[ormsbee]

Anything touching tracking events is higher risk because the generated log data often produces data that is not carefully analyzed until months later. You have these changes as separate commits, which is great. But please separate the static_url commit to its own PR. This PR can be kept for the event tracking/publishing changes, and may require separate testing.

@pdpinch, @ashultz0, @jristau1984: Do you have any concerns/thoughts regarding the validation for the XBlock event publishing/tracking log aspects of this PR?

[ashultz0]

@ormsbee I don't think this touches any of the answer stuff we track for insights

[ormsbee]

@ashultz0: Really? I thought this was how all those events got to the tracking logs in the end...?

[ashultz0]

Maybe I'm missing some of the context because I'm not familiar with this code.

Insights basically only looks at answers (and only our older types of answers), it doesn't engage with any of the completion stuff that seems to be what is being changed here.

Possibly the learner view looked at completion but 1) I don't think it did and 2) I don't have to research it because we are burning it to the ground with community OK anyway.

[Agrendalath]

@ormsbee, thank you for reviewing this. I separated the static_url deprecation to #30992.

@ashultz0, this service also handles events like problem_check (analytics seem to be using it here), showanswer (listed here), etc. This PR should not alter events in any way, but this service will be passing them to the track_function, so it introduces some risk.

[ashultz0]

thanks @Agrendalath

problem_check is the big one for the answers view so we can just validate that they are still happening as expected afterwards

[pkulkark]

I haven't done a deep code review since @ormsbee is doing that. I tested it out on the sandbox and confirm that it's working as described in the testing instructions. LGTM 👍

P.S @Agrendalath I assume you'll be rebasing with latest master once the review is done?

[Agrendalath]

@pkulkark,

P.S @Agrendalath I assume you'll be rebasing with latest master once the review is done?

Of course. Each of these BD-13 will have a conflict with the master branch once the other BD-13 PR is merged.

[Agrendalath]

@ormsbee, what are the next steps for this PR? Are we waiting for more feedback before moving forward?

[ormsbee]

@schenedx, @jristau1984: This was the tracking log related code refactoring PR we discussed today.

[schenedx]

@ormsbee We found the tracking logs are sent to Segment app on edX side. Because of that, we can quickly verify this PR on stage and prod.
Once you feel like this PR is ready, please let me know so we can merge, deploy and test with our people on the look out

[schenedx]

@ormsbee OK, So I made a mistake conceptually. I thought the logs sent to segment app is the same as tracking logs.
This is not the case. Tracking logs are log files written into the disk, and ansible has a role to copy those logs into S3 bucket edx-all-tracking-logs. See reference here.
Segment logs are sent to the segment app directly and segment will store those logs inside of S3 into a different S3 bucket. What I don't know is if these two logs are going to be logging different events, or same events or sometimes different sometimes same.
Let's assume the worst that they are logging different events, I believe this logging is affecting pure tracking logs. In order to see the effect of this change, we'd have to monitor the prod instance tracking log writes unto EC2 disk. That's an SRE thing.
I will help get SRE attention onto this PR.

[e0d]

@ormsbee @schenedx what remains to be reviewed here to get this over the line?

[schenedx]

@e0d I asked @ormsbee to help me validate if the code in this change would touch segment tracking events as well. That said, I also wanted to get 2U SRE to be helping me with testing this change on the stage environment to ensure tracking logs are still created. Cannot validate that with the current access I have.

[schenedx]

@e0d I am planning to merge this PR and validate on stage Monday, Nov 14th. 2022

[schenedx]

LGTM

[openedx-webhooks]

@Agrendalath 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

[edx-pipeline-bot]

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production.

[edx-pipeline-bot]

EdX Release Notice: This PR has been deployed to the production environment.