Skip to content

Conversation

demoncoder-crypto
Copy link

@demoncoder-crypto demoncoder-crypto commented Apr 10, 2025

This pull request modifies the getEventInfoForSpark function within flyteplugins/go/tasks/plugins/k8s/spark/spark.go.
Extract Timestamps: Retrieves the SubmissionTime, CompletionTime, and TerminationTime from the sparkOp.SparkApplicationStatus (sj.Status).
Format Timestamps: The retrieved metav1.Time values are formatted into RFC3339 strings and standard Unix timestamps (int64). SubmissionTime is used for the start time, and CompletionTime (or TerminationTime as a fallback) is used for the end time. Checks are included to handle cases where these timestamps might be zero (e.g., job hasn't started/finished).
Populate tasklog.Input: The formatted timestamps are used to populate the PodRFC3339StartTime, PodRFC3339FinishTime, PodUnixStartTime, and PodUnixFinishTime fields within the tasklog.Input struct.
Targeted Application: This population is specifically done for the calls to p.GetTaskLogs that fetch logs associated with the Spark driver pod (sj.Status.DriverInfo.PodName), namely the "Driver Logs" (using the "Mixed" log config) and "User Logs" (using the "User" log config). Calls for "System" and "AllUser" logs remain unchanged as they use the application name rather than a specific pod name.

Summary by Bito

This PR enhances Spark logging by extracting time metrics from SparkApplicationStatus, computing start/finish timestamps based on submission and completion times. The timestamps are formatted in RFC3339 and Unix formats to improve the tasklog.Input structure for both driver and user logs, enabling more accurate log tracking and troubleshooting.

Unit tests added: False

Estimated effort to review (1-5, lower is better): 1

Copy link

welcome bot commented Apr 10, 2025

Thank you for opening this pull request! 🙌

These tips will help get your PR across the finish line:

  • Most of the repos have a PR template; if not, fill it out to the best of your knowledge.
  • Sign off your commits (Reference: DCO Guide).

@flyte-bot
Copy link
Collaborator

flyte-bot commented Apr 10, 2025

Code Review Agent Run #adc73a

Actionable Suggestions - 0
Review Details
  • Files reviewed - 1 · Commit Range: b23de8d..b23de8d
    • flyteplugins/go/tasks/plugins/k8s/spark/spark.go
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

Refer to the documentation for additional commands.

Configuration

This repository uses code_review_bito You can customize the agent settings here or contact your Bito workspace admin at [email protected].

Documentation & Help

AI Code Review powered by Bito Logo

@demoncoder-crypto
Copy link
Author

Not yet tested I just want to make sure my implementation and my track is correct as this is my first contribution to flyte.

@flyte-bot
Copy link
Collaborator

Changelist by Bito

This pull request implements the following key changes.

Key Change Files Impacted
New Feature - Spark Log Time Population

spark.go - Enhanced getEventInfoForSpark by extracting, formatting, and populating SubmissionTime and Completion/TerminationTime into both RFC3339 and Unix timestamp fields for improved log link functionality.

Copy link

codecov bot commented Apr 26, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 59.46%. Comparing base (156ff48) to head (b23de8d).
Report is 8 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6411      +/-   ##
==========================================
+ Coverage   58.49%   59.46%   +0.97%     
==========================================
  Files         940      553     -387     
  Lines       71555    37862   -33693     
==========================================
- Hits        41855    22515   -19340     
+ Misses      26519    13690   -12829     
+ Partials     3181     1657    -1524     
Flag Coverage Δ
unittests-datacatalog 59.03% <ø> (ø)
unittests-flyteadmin 56.25% <ø> (-0.03%) ⬇️
unittests-flytecopilot 30.99% <ø> (ø)
unittests-flytectl 64.76% <ø> (+0.05%) ⬆️
unittests-flyteidl 76.12% <ø> (ø)
unittests-flyteplugins ?
unittests-flytepropeller ?
unittests-flytestdlib 64.02% <ø> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@kumare3
Copy link
Contributor

kumare3 commented Apr 26, 2025

We will review and get back soon

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.

3 participants