Telemetry Actions not marked as Errors

[dviry]

Maybe I am missing something - but shouldn't Telemetry Actions be set to Status error when... an error occurs?

In my code (in this case a QueueJob handler based on QueueJobBase<>) I had to wrap as following:

protected async override Task<JobResult> ProcessQueueEntryAsync(QueueEntryContext<IdpWorkItem> context)
{
   try
   {
      ......
   }
  catch (Exception ex)
  {
      Activity.Current?.SetStatus(ActivityStatusCode.Error, ex.Message);
      Activity.Current?.RecordException(ex);
      return JobResult.FromException(ex);
   }
}

Wouldn't be better to have that in here:

Foundatio/src/Foundatio/Jobs/QueueJobBase.cs

Line 137 in ae95423

if (!queueEntry.IsCompleted && !queueEntry.IsAbandoned)

Or even better here:

Foundatio/src/Foundatio/Jobs/JobRunner.cs

Line 213 in ae95423

_logger.LogJobResult(result, _options.Name);

I can find multiple places where this could be done (IMO the higher the better) - but is there any reason this was not done originally?

Thanks

[niemyjski]

This is the first time I've come across these api's personally. How is this working out for you, are you able to see each queue attempt failure as an error? What happens when it succeeds after a retry? I think as long as you are marking a non root span as an error you are fine.

What other places would you set this?

[ejsmith]

@dviry I agree that the activities should be marked as error status. Could you submit a PR to make this change?

[dviry]

@niemyjski exactly, each failed attempt is marked as an Error and includes the Exception details - making it very easy to find/analyse in the logs. The successful ones stay as is. Whether the whole Job's activity is then marked as an error or not depends on how we want to handle the "propagation" up (depending on error handling).

@ejsmith I've honestly just started using this library (and for now only using QueueJobs) so I am still trying to find my way around - not yet sure where the best "catch-all" would be, usually everywhere you start an Activitity you finalize it with a status (or error in this case). I do agree with most items planned in #274 - a consolidation of sorts would make it way easier but I guess that will never happen (ticket from 2022)...

I'll be happy to submit a PR once I know what to do where ;)