dotnet / dnceng

.NET Engineering Services
MIT License
23 stars 18 forks source link

Console logged URI's in internal builds are hard to use #1222

Open dotnet-bot opened 2 years ago

dotnet-bot commented 2 years ago

Migrated from https://github.com/dotnet/core-eng/issues/15366

@ChadNedzlek wrote:

Split from https://github.com/dotnet/core-eng/issues/15335

We found some usability issues while working through problems in the https://dev.azure.com/dnceng/internal/_git/dotnet-aspnetcore/pullrequest/20208 pull request. (Background note: Tests are enabled in internal verification builds of PRs like this.)

Taking https://dev.azure.com/dnceng/internal/_build/results?buildId=1551770&view=results as an example:

  1. The Artifacts tab for any failed work item in for example Pipelines - Run 20220113.15 (azure.com) is empty. I'm not sure if the problem is permissions (this is an internal PR after all), core dumps flooding the system, or something else.
  2. Console logs don't exist at all when the Python code fails before opening a console. This makes the failure links in the build step 404.
  3. Separately and someone less annoyingly (since pasting a token is required anyhow), navigating from the work item list provided in the build step logs to a specific details URI means adding &access_token={token} instead of changing {Get to your token.
missymessa commented 2 years ago

Missy to follow up with @ChadNedzlek about this.

ChadNedzlek commented 2 years ago

I don't like the console logged URI's at all. They are a lame way to get at what the artfiacts tab is much better at showing you. Honestly the error should just be "your tests failed, go to the artifacts tab to see details"

missymessa commented 2 years ago

@ChadNedzlek and @dougbu is this still an issue that we need to address?

dougbu commented 2 years ago

I haven't noticed any improvement in this area.

ChadNedzlek commented 2 years ago

Well, my feedback remains the same... all the logged URI's lead to worse UI experiences than using the Tests tab in AzDO, so I'm a fan of removing them in order to push people toward the UI we are focusing on.

missymessa commented 2 years ago

Okay, following up: Chad shared with me a location that has the link: https://github.com/dotnet/arcade/blob/e7ede87875f41a9b3df898ae08da5ebc96e24f56/src/Microsoft.DotNet.Helix/Sdk/CheckHelixJobStatus.cs#L41

The problem is that the code doesn't understand if the URI exists or if it will 404. And if the access token isn't present, users will have to muck around with it to make it work. This is legacy functionality from before a time the Test tab existed. Since it exists now, it would be better to remove the places where we present questionable links and replace it with a comment telling to user to check out the test tab.

@dougbu, you good with us removing these URIs and replacing it with that direction?

dougbu commented 2 years ago

@dougbu, you good with us removing these URIs and replacing it with that direction?

Yup, this works for me. Less links, less confusion đŸ˜„

riarenas commented 2 years ago

Are these the same links as the ones we're saying we'll improve in https://github.com/dotnet/arcade/issues/10375? (just to make sure we're consistent on the messaging) I support the removal of the links.

missymessa commented 2 years ago

Are these the same links as the ones we're saying we'll improve in dotnet/arcade#10375? (just to make sure we're consistent on the messaging) I support the removal of the links.

It doesn't look like they're the same. Chad says the ones in that issue come from this location: https://github.com/dotnet/arcade/blob/main/src/Microsoft.DotNet.Helix/Sdk/tools/Microsoft.DotNet.Helix.Sdk.MonoQueue.targets#L78

riarenas commented 2 years ago

I think that means they are the same.

That other issue is asking for the second part of the logging to also contain the link that the initial log has. as in we're logging

Sent Helix Job; see work items at https://helix.dot.net/api/jobs/e6df725b-7a21-4eab-a4c1-3c1e82c8b4be/workitems?api-version=2019-06-17

and the ask is to also log the link when the work items finish.

Job e6df725b-7a21-4eab-a4c1-3c1e82c8b4be on Windows.11.Amd64.Client.Open is completed with 175 finished work items.
riarenas commented 2 years ago

The funniest scenario would be if we do both issues, and we remove the link from the job start, and add it to the job completed.

missymessa commented 2 years ago

image

missymessa commented 2 years ago

It looks like the url in CheckHelixJobStatus uses a value for Console URI, while the one in .targets file manually constructs it. Is the Console URI the same value as the one that's manually constructed in .targets? Is it possible that the value in the targets file is more likely to be a valid URL verus the one in CheckHelixJobStatus that might not exist (and thus, 404s)?

ChadNedzlek commented 1 year ago

This comes up a lot when doing servicing, since the PR's are in internal. I really feel like we should just delete the links and replace them with "for details, see the test tab". People don't seem to know to go there (and, admittedly, it's not obvious useful information is there).

It came up again in FR today, and it seems to be costing people a fair bit of time in aggregate (them getting lost and confused, then coming to FR, then us trying to investigate why logs are "missing" before we realize it's just this issue again)

missymessa commented 1 year ago

Action item: Remove the links, direct folks to use the Helix Test tab.

Migrating to the Shared Test Infra epic.