Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LogAnalysis should not rely on cell execution being recorded by LogEvents #222

Merged
merged 10 commits into from
Sep 7, 2024

Conversation

jlewi
Copy link
Owner

@jlewi jlewi commented Aug 30, 2024

Now that we no longer need to process RunMe logs we can simplify log processing

  • For the watermark we can just keep track of a single file and its offset
  • Only the latest file will be active. Therefore we don't need to watch the filesystem for modifications
    we can just periodically scan the logs.

Changes to BlockLog Proto

  • We can remove ExecTraceIds
    • Since the AI doesn't handle executions we don't have traces for execution
    • We should rely on the Frontend sending details of execution (e.g. execute code) which it currently isn't
    • Since LogEvents are processed in order we know that the most recent cell execution will be the final one reported by a LogEvent
  • Add a field to record suggestion status
    • Right now we only get log events for cells being accepted we don't record cells being rejected but we can reasonably infer that unaccepted cells have been rejected

Config Changes

  • We can deprecate the logDirs field in learner since users should no longer need to configure it to monitor RunMe Logs
  • Simplifying the setup process of the learner is one of the main motivations of this PR.

Analyzer Changes

*. We no longer need CombineTraces functions for RunMe and Execute traces

  • We just accumulate LogEvents on the BlockLog object as we sequentially process the logs.

Other changes

  • Add an RPC method to check the logs status.
    • This will report the watermark
  • Add ZapProto function to handle logging protos that include RunMe objects
    • We can't rely on the custom go plugin to generate the MarshalObject function because the RunMe protos aren't using that plugin

Related to #211

jlewi added 7 commits August 29, 2024 14:45
…file processing.

* For the watermark we can just keep track of a single file and its offset
* Only the latest file will be active. Therefore we don't need to watch the filesystem for modifications
  we can just periodically scan the logs.

* Add an RPC method to check the logs status.
Copy link

netlify bot commented Aug 30, 2024

Deploy Preview for foyle ready!

Name Link
🔨 Latest commit b47f046
🔍 Latest deploy log https://app.netlify.com/sites/foyle/deploys/66dc2717f7e82a00085df6b6
😎 Deploy Preview https://deploy-preview-222--foyle.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@jlewi jlewi changed the title Jlewi/main LogAnalysis should no rely on cell execution being recorded by LogEvents Aug 30, 2024
@jlewi jlewi changed the title LogAnalysis should no rely on cell execution being recorded by LogEvents LogAnalysis should not rely on cell execution being recorded by LogEvents Sep 2, 2024
@jlewi jlewi marked this pull request as ready for review September 7, 2024 10:35
@jlewi jlewi merged commit 9d43ef9 into main Sep 7, 2024
5 checks passed
@jlewi jlewi deleted the jlewi/main branch September 7, 2024 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant