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.

Sign up for GitHub

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

Jump to bottom

Gitlab collapsible trace reporter #2477

Merged
merged 2 commits into from Mar 5, 2020
Merged

Gitlab collapsible trace reporter #2477

merged 2 commits into from Mar 5, 2020

Conversation

baronfel
Copy link
Contributor

@baronfel baronfel commented Mar 5, 2020 •
edited

Description

Implements the new collapsible-section logging for Gitlab CI runners as described in the docs.

TODO:

  • boy scout rule: "leave the code behind in a better state than you found it" (fix warnings, obsolete members or code-style in the places you worked in)
  • Fake 5 API guideline is honored

@baronfel baronfel force-pushed the gitlab-collapsible-trace-reporter branch from 8c8bcf7 to 0cff0d1 Compare March 5, 2020 15:57
baronfel
baronfel commented Mar 5, 2020
View reviewed changes
src/app/Fake.BuildServer.GitLab/GitLab.fs
| TraceData.CloseTag (tag, time, state) ->
write false color true (sprintf "Finished (%A) '%s' in %O" state tag.Name time)
let unixTimestamp = System.DateTimeOffset.UtcNow.ToUnixTimeSeconds()
let sectionFooter = sprintf "section_end:%d:%s_%s\r\e[0K" unixTimestamp tag.Type tag.Name
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried to use the time field here, but the problem with that is that the expected format of the closing tag doesn't want the duration, it just wants the unix ticks. The TraceData types might be changed to include timestamp information (ie OpenTag gains a DateTimeOffset for the start, and CloseTag gains a DateTimeOffset and TimeSpan for the stop and duration respectively) and then this output could be fully-deterministic. I'm not sure if you want to make that kind of a change or not.

Copy link
Member

Choose a reason for hiding this comment

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

I don't mind it as-is. I'm not sure what you mean with "fully-deterministic". That output will never be deterministic as the time will always vary, correct?
So I don't see any gain from breaking anything here?
I guess you mean deterministic across different loggers?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just meant that we have a dependency on DateTimeOffset.UtcNow now, instead of that data being provided by the tracing subsystem. There's a time difference between when the trace message is fired and when the listener gets the message, so some message fidelity is lost. It's probably not a big deal, because the time reporting in gitlab is at the resolution of seconds, not milliseconds, but I just wanted to bring it up.

@matthid matthid merged commit 9937ac5 into fsprojects:release/next Mar 5, 2020
@matthid matthid mentioned this pull request Mar 5, 2020
Merged
3 tasks
@baronfel
Copy link
Contributor Author

baronfel commented Mar 5, 2020

I've updated the work builds with the latest alpha and there are problems with the formatting, so I'll be sending another MR soon.

@baronfel
Copy link
Contributor Author

baronfel commented Mar 5, 2020

Sample written output after this change:

�[37;2msection_start:1583446351:target_Clean
\e[0K�[0m
�[37;2msection_start:1583446351:target_ReplaceTemplateFilesNamespace
\e[0K�[0m
�[0Ksection_start:1583446308:restore_cache
�[0Ksection_end:1583446309:restore_cache
�[0Ksection_start:1583446309:download_artifacts
�[0Ksection_end:1583446311:download_artifacts
�[0Ksection_start:1583446311:build_script

Problems I see:

  • newlines in the tag descriptions
  • ansi escape codes from the ansi color output writer

@baronfel baronfel deleted the gitlab-collapsible-trace-reporter branch March 5, 2020 22:30
@baronfel baronfel mentioned this pull request Mar 5, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matthid matthid matthid left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Gitlab Integration] Support nested sections in the build log
2 participants
@baronfel @matthid