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

Add task start and end timestamps #50

Merged
merged 1 commit into from
Jun 19, 2024

Conversation

eduardb
Copy link
Contributor

@eduardb eduardb commented Jun 14, 2024

Hello there!
I am planning to use this plugin for pushing build telemetry into our observability platform. However, in order to be able to build the tasks as a tree of spans, we need the start and end timestamps for them. So I am adding these fields to MeasuredTask.
Please let me know if there's any kind of improvement you'd like to see in this PR to make it shippable. Cheers!

@@ -138,6 +138,7 @@ class BuildTimePluginTest {
assertThat(executionData.executedTasks).hasSize(1).first().satisfies({ task ->
assertThat(task.name).isEqualTo(":help")
assertThat(task.state).isEqualTo(MeasuredTask.State.EXECUTED)
assertThat(task.endTimestamp - task.startTimestamp).isEqualTo(task.duration.inWholeMilliseconds)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Arguably, this assertion shouldn't be part of this test in particular, but it was quite convenient. But I can write a new dedicated test if you insist.

Copy link
Member

@wzieba wzieba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution @eduardb ! The MeasuredTask#duration is now functionally dependent from startTimestamp and endTimestamp, but I think this is a minor issue (or not an issue at all).

@wzieba wzieba merged commit 9869f17 into Automattic:main Jun 19, 2024
2 checks passed
@eduardb
Copy link
Contributor Author

eduardb commented Jun 19, 2024

The MeasuredTask#duration is now functionally dependent from startTimestamp and endTimestamp, but I think this is a minor issue (or not an issue at all).

Oh, that's a good point. Perhaps only startTimestamp should be added, and the end timestamp can be calculated by whoever needs it by adding duration to it? Let me know if you'd like me to quickly change this.

@eduardb
Copy link
Contributor Author

eduardb commented Jun 19, 2024

Thanks for merging, btw! ❤️
Are you also planning on doing a release with this change? :D

@eduardb eduardb deleted the add-task-start-and-end-timestamps branch June 19, 2024 11:01
@wzieba
Copy link
Member

wzieba commented Jun 19, 2024

I already started release of 3.1.0, but it's not yet published on maven central gradle plugin portal.

Perhaps only startTimestamp should be added, and the end timestamp can be calculated by whoever needs it by adding duration to it? Let me know if you'd like me to quickly change this.

Actually, that'd be very nice - could you please prepare a PR and then I'll include it in 3.1.0 release? 🙂

@eduardb
Copy link
Contributor Author

eduardb commented Jun 19, 2024

Done here: #51

@wzieba
Copy link
Member

wzieba commented Jun 19, 2024

Thank you for the follow up PR @eduardb . Publishing to the gradle plugin portal was successful, 3.1.0 should soon be available.

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.

2 participants