Skip to content
This repository has been archived by the owner on Nov 1, 2023. It is now read-only.

add log checking to refactored integration check #700

Merged
17 commits merged into from
Apr 2, 2021

Conversation

bmc-msft
Copy link
Contributor

@bmc-msft bmc-msft commented Mar 19, 2021

In practice, Application Insights can take up to 3 minutes before something sent to it is available via KQL.

This PR logs a start and stop marker such that the integration tests only search for logs during the integration tests. This reduces the complexity when using the integration tests during the development process.

Note: this migrated the new functionality from #356 into the latest integration test tools.

@bmc-msft bmc-msft marked this pull request as draft March 19, 2021 18:16
@@ -214,9 +214,6 @@ def on_worker_event_done(machine_id: UUID, event: WorkerDoneEvent) -> Result[Non
node.debug_keep_node = True
node.save()
else:
logging.error(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

note, this is now handled by task.mark_failed. Since we don't record errors on tasks already marked as failures, this moves the error longing to model our state handling.

@@ -136,7 +136,7 @@ def get_extension(vm_name: str, extension_name: str) -> Optional[Any]:
resource_group, vm_name, extension_name
)
except (ResourceNotFoundError, CloudError) as err:
logging.error("extension does not exist %s", err)
logging.info("extension does not exist %s", err)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is common place when we're deploying VMs. shouldn't be an error.

notifications = get_notifications(container)

report = get_report_or_regression(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

since we're going to try to inspect every new file as a potential report, this sets up the parsing (seen later in this PR) to only log errors if we actually expect the files to be a report, rather than just optimistically being a report.

@@ -205,6 +205,8 @@ def mark_failed(
)
return

logging.error("task failed %s:%s - %s", self.job_id, self.task_id, error)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

See previous comment about mark_failed.

@bmc-msft bmc-msft marked this pull request as ready for review March 19, 2021 21:59
@bmc-msft bmc-msft linked an issue Mar 19, 2021 that may be closed by this pull request
@bmc-msft bmc-msft requested a review from chkeita March 23, 2021 02:47
@ghost
Copy link

ghost commented Apr 2, 2021

Hello @bmc-msft!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost ghost merged commit ca12904 into microsoft:main Apr 2, 2021
@ghost ghost locked as resolved and limited conversation to collaborators May 3, 2021
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add app-insights queries to integration tests
3 participants