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

Job Log Model #1030

Merged
merged 30 commits into from Dec 13, 2021
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
2d752eb
Create JobLogEntry and migrations.
smk4664 Oct 25, 2021
b3aa0b0
Remove functions and documentation that is no longer relevent.
smk4664 Oct 25, 2021
99676bd
Fix tests and allow JobLogEntries to be created outside transaction.a…
smk4664 Oct 25, 2021
0deff07
Convert views to use generic.objectview and django-tables2.
smk4664 Oct 25, 2021
45dbaf4
Correct git repository deletion bug.
smk4664 Oct 25, 2021
31a72b4
Update nautobot/docs/additional-features/jobs.md
smk4664 Oct 25, 2021
121a62f
Update nautobot/docs/models/extras/jobresult.md
smk4664 Oct 25, 2021
31b9f94
Update nautobot/docs/additional-features/jobs.md
smk4664 Oct 25, 2021
0bc4bbb
Update nautobot/extras/datasources/git.py
smk4664 Oct 25, 2021
7f7720e
Address page span issue with Additonal Data.
smk4664 Oct 25, 2021
d48404a
Add truncating of object strings to the JobLogEntry.
smk4664 Oct 25, 2021
7928e4a
Address the rest of the feedback.
smk4664 Oct 25, 2021
7c72484
Remove unused imports.
smk4664 Oct 25, 2021
6fec78c
Merge branch 'next' of github.com:nautobot/nautobot into JobResults
smk4664 Nov 20, 2021
ad5788c
Updating Migration order after upstream changes.
smk4664 Nov 20, 2021
8e09e96
Address feedback from PR.
smk4664 Nov 22, 2021
ed9e2aa
Black recent changes.
smk4664 Nov 22, 2021
a4d9615
Update nautobot/docs/models/extras/jobresult.md
smk4664 Nov 22, 2021
ed47263
Update nautobot/extras/models/models.py
smk4664 Nov 22, 2021
d7fc021
Update nautobot/extras/models/models.py
smk4664 Nov 22, 2021
7e5083e
Make additional changes recommended by peer review.
smk4664 Nov 22, 2021
332ba95
Display error message if the job log table is not available.
smk4664 Nov 29, 2021
d0f0585
Update nautobot/extras/templates/extras/jobresult.html
smk4664 Dec 6, 2021
9ed41ca
Update nautobot/extras/tests/test_models.py
smk4664 Dec 6, 2021
0c0e032
Address hanging div's and add back active_tab.
smk4664 Dec 6, 2021
eef30c0
Update nautobot/docs/models/extras/joblogentry.md
smk4664 Dec 13, 2021
721953e
Update nautobot/docs/models/extras/jobresult.md
smk4664 Dec 13, 2021
4563c4a
Update nautobot/docs/models/extras/joblogentry.md
smk4664 Dec 13, 2021
4819cfe
Prepend constants with JOB_LOG_
smk4664 Dec 13, 2021
6561bbb
Black
smk4664 Dec 13, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions nautobot/core/cli.py
Expand Up @@ -132,6 +132,10 @@ def _configure_settings(config):
if settings.METRICS_ENABLED and "postgres" in settings.DATABASES["default"]["ENGINE"]:
settings.DATABASES["default"]["ENGINE"] = "django_prometheus.db.backends.postgresql"

# Create fake db for job logs. This uses the default db, but allows us to save logs within
# transaction.atomic().
settings.DATABASES["job_logs"] = settings.DATABASES["default"]
Copy link
Contributor

Choose a reason for hiding this comment

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

This worries me in that it is a magic setting that we're trying to move away from.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, if we move away from running jobs in Atomic Transactions then this can go away as well. This is not meant to be modified, so that is why we stuck it here.


#
# Media storage
#
Expand Down
5 changes: 2 additions & 3 deletions nautobot/docs/additional-features/jobs.md
Expand Up @@ -289,11 +289,11 @@ Log messages generated by any of these methods will be automatically grouped tog

### The `post_run()` Method

If your job class implements a `post_run()` method (which must take no arguments other than `self`), this method will be automatically invoked after the `run()` and `test_*()` methods (if any). It will be called even if one of the other methods raises an exception, so this method can be used to handle any necessary cleanup or final events (such as sending an email or triggering a webhook). The status of the overall job is available at this time as `self.failed` and the [`JobResult`](../models/extras/jobresult.md) data object is available as `self.result`.
If your job class implements a `post_run()` method (which must take no arguments other than `self`), this method will be automatically invoked after the `run()` and `test_*()` methods (if any). It will be called even if one of the other methods raises an exception, so this method can be used to handle any necessary cleanup or final events (such as sending an email or triggering a webhook). The status of the overall job is available at this time as `self.failed` and the associated [`JobResult`](../models/extras/jobresult.md) `data` field is available as `self.results`.

### Logging

The following instance methods are available to log results from an executing job to be stored into the associated [`JobResult`](../models/extras/jobresult.md) record:
The following instance methods are available to log results from an executing job to be stored into [`JobLogEntry`](../models/extras/joblogentry.md) records associated with the current [`JobResult`](../models/extras/jobresult.md):

* `self.log(message)`
* `self.log_debug(message)`
Expand All @@ -311,7 +311,6 @@ Markdown rendering is supported for log messages.
!!! note
Using `self.log_failure()`, in addition to recording a log message, will flag the overall job as failed, but it will **not** stop the execution of the job. To end a job early, you can use a Python `raise` or `return` as appropriate.


### Accessing Request Data

Details of the current HTTP request (the one being made to execute the job) are available as the instance attribute `self.request`. This can be used to infer, for example, the user executing the job and their client IP address:
Expand Down
11 changes: 11 additions & 0 deletions nautobot/docs/models/extras/joblogentry.md
@@ -0,0 +1,11 @@
# Job Log Entry

As of Nautobot 1.2, log messages from jobs are now stored in the JobLogEntry model. This allows more performant querying of log messages and even allows viewing of logs while the job is still running.
smk4664 marked this conversation as resolved.
Show resolved Hide resolved

Records of this type store the following data:

- A reference to the JobResult object that created the log.
smk4664 marked this conversation as resolved.
Show resolved Hide resolved
- Timestamps indicating when the log message was created.
- The logging level of the log message.
- The log message.
- If provided, the string format of the logged object and it's absolute url.
2 changes: 1 addition & 1 deletion nautobot/docs/models/extras/jobresult.md
Expand Up @@ -8,4 +8,4 @@ Records of this type store the following data:
- A reference to the user who initiated the task
- Timestamps indicating when the task was created and when it completed
- An overall status such as "pending", "running", "errored", or "completed".
- A block of structured data (often rendered as JSON); by convention this block is used to store detailed log records of the task execution, but it may in theory be used (such as by a plugin) to store other forms of structured data as well.
- A block of structured data (often rendered as JSON); Any return values from the `.run()` and any `test` methods go to `output`. In addition any Job or plugin using the JobResult model can store arbitrary structured data here if needed. (Note that prior to Nautobot 1.2, job log records were stored in this field; they are now stored as distinct [`JobLogEntry`](joblogentry.md) records instead.)
smk4664 marked this conversation as resolved.
Show resolved Hide resolved
5 changes: 5 additions & 0 deletions nautobot/extras/constants.py
Expand Up @@ -15,3 +15,8 @@
"statuses",
"webhooks",
]

# JobLogEntry Truncation Length
MAX_GROUPING_LENGTH = 100
MAX_LOG_OBJECT_LENGTH = 200
MAX_ABSOLUTE_URL_LENGTH = 255
smk4664 marked this conversation as resolved.
Show resolved Hide resolved
3 changes: 2 additions & 1 deletion nautobot/extras/datasources/git.py
Expand Up @@ -27,6 +27,7 @@
ConfigContextSchema,
ExportTemplate,
GitRepository,
JobLogEntry,
JobResult,
Tag,
)
Expand Down Expand Up @@ -108,7 +109,7 @@ def pull_git_repository_and_refresh_data(repository_pk, request, job_result_pk):

finally:
if job_result.status not in JobResultStatusChoices.TERMINAL_STATE_CHOICES:
if job_result.data["total"][LogLevelChoices.LOG_FAILURE] > 0:
if JobLogEntry.objects.filter(job_result__pk=job_result.pk, log_level=LogLevelChoices.LOG_FAILURE).exists():
job_result.set_status(JobResultStatusChoices.STATUS_FAILED)
else:
job_result.set_status(JobResultStatusChoices.STATUS_COMPLETED)
Expand Down
16 changes: 13 additions & 3 deletions nautobot/extras/datasources/registry.py
Expand Up @@ -19,7 +19,7 @@ def get_datasource_content_choices(model_name):
)


def refresh_datasource_content(model_name, record, request, job_result, delete=False):
def refresh_datasource_content(model_name, record, request, job_result, delete=False, use_default_db=False):
"""Invoke the refresh callbacks for every content type registered for this model.

Note that these callback functions are invoked regardless of whether a given model instance actually is flagged
Expand All @@ -32,44 +32,54 @@ def refresh_datasource_content(model_name, record, request, job_result, delete=F
request (HttpRequest): Initiating request for this refresh, optional, used for change logging if provided
job_result (JobResult): Passed through to the callback functions to use with logging their actions.
delete (bool): True if the record is being deleted; False if it is being created/updated.
use_default_db(bool): Logs created should use the default database instead of the "job_logs" database.
"""
job_result.log(
f"Refreshing data provided by {record}...",
level_choice=LogLevelChoices.LOG_INFO,
use_default_db=use_default_db,
)
job_result.save()
if request:
with change_logging(request):
for entry in get_datasource_contents(model_name):
job_result.log(f"Refreshing {entry.name}...", level_choice=LogLevelChoices.LOG_INFO)
job_result.log(
f"Refreshing {entry.name}...", level_choice=LogLevelChoices.LOG_INFO, use_default_db=use_default_db
)
try:
entry.callback(record, job_result, delete=delete)
except Exception as exc:
job_result.log(
f"Error while refreshing {entry.name}: {exc}",
level_choice=LogLevelChoices.LOG_FAILURE,
use_default_db=use_default_db,
)
job_result.set_status(JobResultStatusChoices.STATUS_ERRORED)
job_result.save()
job_result.log(
f"Data refresh from {record} complete!",
level_choice=LogLevelChoices.LOG_INFO,
use_default_db=use_default_db,
)
job_result.save()
else:
for entry in get_datasource_contents(model_name):
job_result.log(f"Refreshing {entry.name}...", level_choice=LogLevelChoices.LOG_INFO)
job_result.log(
f"Refreshing {entry.name}...", level_choice=LogLevelChoices.LOG_INFO, use_default_db=use_default_db
)
try:
entry.callback(record, job_result, delete=delete)
except Exception as exc:
job_result.log(
f"Error while refreshing {entry.name}: {exc}",
level_choice=LogLevelChoices.LOG_FAILURE,
use_default_db=use_default_db,
)
job_result.set_status(JobResultStatusChoices.STATUS_ERRORED)
job_result.save()
job_result.log(
f"Data refresh from {record} complete!",
level_choice=LogLevelChoices.LOG_INFO,
use_default_db=use_default_db,
)
job_result.save()
69 changes: 7 additions & 62 deletions nautobot/extras/jobs.py
Expand Up @@ -98,23 +98,11 @@ class Meta:

pass

@staticmethod
def _results_struct():
return OrderedDict(
[
("success", 0),
("info", 0),
("warning", 0),
("failure", 0),
("log", []),
]
)

def __init__(self):
self.logger = logging.getLogger(f"nautobot.jobs.{self.__class__.__name__}")

self.request = None
self.active_test = None
self.active_test = "main"
self.failed = False
self._job_result = None

Expand Down Expand Up @@ -224,63 +212,20 @@ def job_result(self):
def job_result(self, value):
# Initialize job_result data format for our usage
value.data = OrderedDict()
value.data["total"] = self._results_struct()
del value.data["total"]["log"]
for method_name in self.test_methods:
value.data[method_name] = self._results_struct()
# Only initialize results for run and post_run if they're actually implemented
if self.run.__func__ != BaseJob.run:
value.data["run"] = self._results_struct()
if self.post_run.__func__ != BaseJob.post_run:
value.data["post_run"] = self._results_struct()

self._job_result = value

@property
def results(self):
"""
The results (log messages and final output) generated by this job.
The results generated by this job.
** If you need the logs, you will need to filter on JobLogEntry **
Ex.
from nautobot.extras.models import JogLogEntry

JobLogEntry.objects.filter(job_result=self.job_result, <other criteria>)

{
"total": {
"success": 0,
"info": 3,
"warning": 6,
"failure": 9,
}
"run": {
"success": 0,
"info": 1,
"warning": 2,
"failure": 3,
"log": [
(timestamp, level, object_name, object_url, message),
(timestamp, level, object_name, object_url, message),
...
],
},
"test_function": {
"success": 0,
"info": 1,
"warning": 2,
"failure": 3,
"log": [
(timestamp, level, object_name, object_url, message),
(timestamp, level, object_name, object_url, message),
...
],
},
"post_run": {
"success": 0,
"info": 1,
"warning": 2,
"failure": 3,
"log": [
(timestamp, level, object_name, object_url, message),
(timestamp, level, object_name, object_url, message),
...
],
},
"output": "...",
}
"""
Expand Down
35 changes: 19 additions & 16 deletions nautobot/extras/management/commands/runjob.py
Expand Up @@ -7,8 +7,8 @@
from django.test.client import RequestFactory
from django.utils import timezone

from nautobot.extras.choices import JobResultStatusChoices
from nautobot.extras.models import JobResult
from nautobot.extras.choices import LogLevelChoices, JobResultStatusChoices
from nautobot.extras.models import JobLogEntry, JobResult
from nautobot.extras.jobs import get_job, run_job
from nautobot.utilities.utils import copy_safe_request

Expand Down Expand Up @@ -74,23 +74,26 @@ def handle(self, *args, **options):
job_result = JobResult.objects.get(pk=job_result.pk)

# Report on success/failure
for test_name, attrs in job_result.data.items():

if test_name in ["total", "output"]:
continue
groups = set(JobLogEntry.objects.filter(job_result=job_result).values_list("grouping", flat=True))
for group in sorted(groups):
logs = JobLogEntry.objects.filter(job_result__pk=job_result.pk, grouping=group)
Comment on lines +77 to +79
Copy link
Contributor

Choose a reason for hiding this comment

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

This smells like possible bad query performance to me. Have you considered the following?

  • Ordering the query by grouping to eliminate the need to use sorted(groups)?
  • Using prefetch_related on job_result to get all of these objects in a single query, instead of round trips to the database for each group?

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 was asked to add sorting to as a set does not have an order. I could change the filter, but it would still be in an unordered set. @glennmatthews was concerned about the sorting.

I will look at the prefetch_related.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also to note, that this is only for running the jobs from the CLI, which I would think is rare.

success_count = logs.filter(log_level=LogLevelChoices.LOG_SUCCESS).count()
info_count = logs.filter(log_level=LogLevelChoices.LOG_INFO).count()
warning_count = logs.filter(log_level=LogLevelChoices.LOG_WARNING).count()
failure_count = logs.filter(log_level=LogLevelChoices.LOG_FAILURE).count()

self.stdout.write(
"\t{}: {} success, {} info, {} warning, {} failure".format(
test_name,
attrs["success"],
attrs["info"],
attrs["warning"],
attrs["failure"],
group,
success_count,
info_count,
warning_count,
failure_count,
)
)

for log_entry in attrs["log"]:
status = log_entry[1]
for log_entry in logs:
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider using logs.iterator() here since these are single-use instances that you're not mutating.

Copy link
Contributor

Choose a reason for hiding this comment

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

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 will look into iterator, but this is only used when running jobs through management commands, which has limited use as they can't take arguments.

status = log_entry.log_level
if status == "success":
status = self.style.SUCCESS(status)
elif status == "info":
Expand All @@ -100,10 +103,10 @@ def handle(self, *args, **options):
elif status == "failure":
status = self.style.NOTICE(status)

if log_entry[2]: # object associated with log entry
self.stdout.write(f"\t\t{status}: {log_entry[2]}: {log_entry[-1]}")
if log_entry.log_object:
self.stdout.write(f"\t\t{status}: {log_entry.log_object}: {log_entry.message}")
glennmatthews marked this conversation as resolved.
Show resolved Hide resolved
else:
self.stdout.write(f"\t\t{status}: {log_entry[-1]}")
self.stdout.write(f"\t\t{status}: {log_entry.message}")

if job_result.data["output"]:
self.stdout.write(job_result.data["output"])
Expand Down
37 changes: 37 additions & 0 deletions nautobot/extras/migrations/0017_joblogentry.py
@@ -0,0 +1,37 @@
# Generated by Django 3.1.13 on 2021-10-18 02:47

from django.db import migrations, models
import django.db.models.deletion
import django.utils.timezone
import uuid


class Migration(migrations.Migration):

dependencies = [
("extras", "0016_secret"),
]

operations = [
migrations.CreateModel(
name="JobLogEntry",
fields=[
(
"id",
models.UUIDField(
default=uuid.uuid4, editable=False, primary_key=True, serialize=False, unique=True
),
),
("log_level", models.CharField(default="default", max_length=32)),
("grouping", models.CharField(default="main", max_length=100)),
("message", models.TextField(blank=True)),
("created", models.DateTimeField(default=django.utils.timezone.now)),
("log_object", models.CharField(blank=True, max_length=200, null=True)),
("absolute_url", models.CharField(blank=True, max_length=255, null=True)),
("job_result", models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to="extras.jobresult")),
],
options={
"abstract": False,
},
),
]