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

Store build.log in log directory #818

Merged
merged 4 commits into from
Nov 19, 2019
Merged

Store build.log in log directory #818

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
823b1e5
Store build.log in log directory
danielmitterdorfer Nov 18, 2019
679aef8
Fix existing pylint errors in code we touched
danielmitterdorfer Nov 18, 2019
c4d2574
Merge remote-tracking branch 'origin/master' into move-build-log
danielmitterdorfer Nov 19, 2019
dd68f38
Fix logrotate comments
danielmitterdorfer Nov 19, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions docs/configuration.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,18 @@ The log file will not be rotated automatically as this is problematic due to Ral
notifempty # don't attempt to rotate empty ones
}

If you are benchmarking source builds of Elasticsearch, Rally will write the log output of the build to ``~/.rally/logs/build.log``. Similarly to the configuration for ``rally.log``, you can use the following ``logrotate`` configuration as a starting point::
Copy link
Contributor

Choose a reason for hiding this comment

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

When reading this first time, it may not be entirely clear what the logrotate config is for, I suggest we rephrase:

you can use the following logrotate configuration as a starting point::

to

you can use the following logrotate configuration to restrict the volume of the log file::

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought that can be inferred based on context (i.e. reading the paragraph above about rally.log).


/home/user/.rally/logs/build.log {
daily # rotate daily
Copy link
Contributor

Choose a reason for hiding this comment

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

In-line comments don't work. They should be in a separate line.

(From man logrotate

Note that comments may appear anywhere in the config file as long as the first non-whitespace character on the line is a #.
)

We need to update also the example for rally.log.

Copy link
Member Author

Choose a reason for hiding this comment

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

addressed in dd68f38.

rotate 7 # keep the last seven log files
maxage 14 # remove logs older than 14 days
compress # compress old logs ...
delaycompress # ... after moving them
missingok # ignore missing log files
notifempty # don't attempt to rotate empty ones
}

Example
~~~~~~~

Expand Down
15 changes: 15 additions & 0 deletions docs/migrate.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,21 @@ Migration Guide
Migrating to Rally 1.4.0
------------------------

Build logs are stored in Rally's log directory
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

If you benchmark source builds of Elasticsearch, Rally has previously stored the build output log in a race-specific directory. With this release, Rally will store build logs in ``/home/user/.rally/logs/build.log``. We recommend to rotate logs with `logrotate <https://linux.die.net/man/8/logrotate>`_. See the following example as a starting point for your own ``logrotate`` configuration and ensure to replace the path ``/home/user/.rally/logs/build.log`` with the proper one::

/home/user/.rally/logs/build.log {
daily # rotate daily
rotate 7 # keep the last seven log files
maxage 14 # remove logs older than 14 days
compress # compress old logs ...
delaycompress # ... after moving them
missingok # ignore missing log files
notifempty # don't attempt to rotate empty ones
}

Index size and Total Written are not included in the command line report
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Expand Down
15 changes: 5 additions & 10 deletions esrally/log.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,11 @@
import os
import time

from esrally import paths
from esrally.utils import io


# pylint: disable=unused-argument
def configure_utc_formatter(*args, **kwargs):
"""
Logging formatter that renders timestamps in UTC to ensure consistent
Expand All @@ -42,13 +44,6 @@ def log_config_path():
return os.path.join(os.path.expanduser("~"), ".rally", "logging.json")


def default_log_path():
"""
:return: The absolute path to the directory that contains Rally's log file.
"""
return os.path.join(os.path.expanduser("~"), ".rally", "logs")


def remove_obsolete_default_log_config():
"""
Log rotation is problematic because Rally uses multiple processes and there is a lurking race condition when
Expand All @@ -61,7 +56,7 @@ def remove_obsolete_default_log_config():
if io.exists(log_config):
source_path = io.normalize_path(os.path.join(os.path.dirname(__file__), "resources", "logging_1_0_0.json"))
with open(source_path, "r", encoding="UTF-8") as src:
contents = src.read().replace("${LOG_PATH}", default_log_path())
contents = src.read().replace("${LOG_PATH}", paths.logs())
source_hash = hashlib.sha512(contents.encode()).hexdigest()
with open(log_config, "r", encoding="UTF-8") as target:
target_hash = hashlib.sha512(target.read().encode()).hexdigest()
Expand All @@ -83,9 +78,9 @@ def install_default_log_config():
source_path = io.normalize_path(os.path.join(os.path.dirname(__file__), "resources", "logging.json"))
with open(log_config, "w", encoding="UTF-8") as target:
with open(source_path, "r", encoding="UTF-8") as src:
contents = src.read().replace("${LOG_PATH}", default_log_path())
contents = src.read().replace("${LOG_PATH}", paths.logs())
target.write(contents)
io.ensure_dir(default_log_path())
io.ensure_dir(paths.logs())


def load_configuration():
Expand Down
17 changes: 8 additions & 9 deletions esrally/mechanic/mechanic.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,9 @@


def download(cfg):
challenge_root_path = paths.race_root(cfg)
car, plugins = load_team(cfg, external=False)

s = supplier.create(cfg, sources=False, distribution=True, build=False,
challenge_root_path=challenge_root_path, car=car, plugins=plugins)
s = supplier.create(cfg, sources=False, distribution=True, build=False, car=car, plugins=plugins)
binaries = s()
console.println(json.dumps(binaries, indent=2), force=True)

Expand Down Expand Up @@ -503,18 +501,19 @@ def load_team(cfg, external):
return car, plugins


def create(cfg, metrics_store, all_node_ips, all_node_ids, cluster_settings=None, sources=False, build=False, distribution=False, external=False,
docker=False):
def create(cfg, metrics_store, all_node_ips, all_node_ids, cluster_settings=None, sources=False, build=False,
distribution=False, external=False, docker=False):
races_root = paths.races_root(cfg)
challenge_root_path = paths.race_root(cfg)
race_root_path = paths.race_root(cfg)
node_ids = cfg.opts("provisioning", "node.ids", mandatory=False)
car, plugins = load_team(cfg, external)

if sources or distribution:
s = supplier.create(cfg, sources, distribution, build, challenge_root_path, car, plugins)
s = supplier.create(cfg, sources, distribution, build, car, plugins)
p = []
for node_id in node_ids:
p.append(provisioner.local_provisioner(cfg, car, plugins, cluster_settings, all_node_ips, all_node_ids, challenge_root_path, node_id))
p.append(
provisioner.local_provisioner(cfg, car, plugins, cluster_settings, all_node_ips, all_node_ids, race_root_path, node_id))
l = launcher.ProcessLauncher(cfg, metrics_store, races_root)
elif external:
raise exceptions.RallyAssertionError("Externally provisioned clusters should not need to be managed by Rally's mechanic")
Expand All @@ -525,7 +524,7 @@ def create(cfg, metrics_store, all_node_ips, all_node_ids, cluster_settings=None
s = lambda: None
p = []
for node_id in node_ids:
p.append(provisioner.docker_provisioner(cfg, car, cluster_settings, challenge_root_path, node_id))
p.append(provisioner.docker_provisioner(cfg, car, cluster_settings, race_root_path, node_id))
l = launcher.DockerLauncher(cfg, metrics_store)
else:
# It is a programmer error (and not a user error) if this function is called with wrong parameters
Expand Down
6 changes: 3 additions & 3 deletions esrally/mechanic/supplier.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,15 @@
import re
import urllib.error

from esrally import exceptions, PROGRAM_NAME
from esrally import exceptions, paths, PROGRAM_NAME
from esrally.exceptions import BuildError, SystemSetupError
from esrally.utils import git, io, process, net, jvm, convert, sysstats

# e.g. my-plugin:current - we cannot simply use String#split(":") as this would not work for timestamp-based revisions
REVISION_PATTERN = r"(\w.*?):(.*)"


def create(cfg, sources, distribution, build, challenge_root_path, car, plugins=None):
def create(cfg, sources, distribution, build, car, plugins=None):
logger = logging.getLogger(__name__)
if plugins is None:
plugins = []
Expand All @@ -43,7 +43,7 @@ def create(cfg, sources, distribution, build, challenge_root_path, car, plugins=
if build_needed:
java_home = _java_home(car)
es_src_dir = os.path.join(_src_dir(cfg), _config_value(src_config, "elasticsearch.src.subdir"))
builder = Builder(es_src_dir, java_home, challenge_root_path)
builder = Builder(es_src_dir, java_home, paths.logs())
else:
builder = None

Expand Down
6 changes: 6 additions & 0 deletions esrally/paths.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,9 @@ def race_root(cfg=None, race_id=None):
race_id = cfg.opts("system", "race.id")
return os.path.join(races_root(cfg), race_id)


def logs():
"""
:return: The absolute path to the directory that contains Rally's log file.
"""
return os.path.join(os.path.expanduser("~"), ".rally", "logs")
4 changes: 2 additions & 2 deletions esrally/rally.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -420,11 +420,11 @@ def print_help_on_errors():
heading = "Getting further help:"
console.println(console.format.bold(heading))
console.println(console.format.underline_for(heading))
console.println("* Check the log files in {} for errors.".format(log.default_log_path()))
console.println("* Check the log files in {} for errors.".format(paths.logs()))
console.println("* Read the documentation at {}".format(console.format.link(doc_link())))
console.println("* Ask a question on the forum at {}".format(console.format.link("https://discuss.elastic.co/c/elasticsearch/rally")))
console.println("* Raise an issue at {} and include the log files in {}."
.format(console.format.link("https://github.com/elastic/rally/issues"), log.default_log_path()))
.format(console.format.link("https://github.com/elastic/rally/issues"), paths.logs()))


def race(cfg):
Expand Down
10 changes: 5 additions & 5 deletions tests/mechanic/supplier_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,7 @@ def test_create_suppliers_for_es_only_config(self):

car = team.Car("default", root_path=None, config_paths=[])

composite_supplier = supplier.create(cfg, sources=False, distribution=True, build=False, challenge_root_path="/", car=car)
composite_supplier = supplier.create(cfg, sources=False, distribution=True, build=False, car=car)

self.assertEqual(1, len(composite_supplier.suppliers))
self.assertIsInstance(composite_supplier.suppliers[0], supplier.ElasticsearchDistributionSupplier)
Expand All @@ -414,7 +414,7 @@ def test_create_suppliers_for_es_distribution_plugin_source_skip(self):
external_plugin = team.PluginDescriptor("community-plugin", core_plugin=False, variables={"enabled": True})

# --pipeline=from-sources-skip-build
composite_supplier = supplier.create(cfg, sources=True, distribution=False, build=False, challenge_root_path="/", car=car, plugins=[
composite_supplier = supplier.create(cfg, sources=True, distribution=False, build=False, car=car, plugins=[
core_plugin,
external_plugin
])
Expand Down Expand Up @@ -446,7 +446,7 @@ def test_create_suppliers_for_es_missing_distribution_plugin_source_skip(self):

# --from-sources-skip-build --revision="community-plugin:current" (distribution version is missing!)
with self.assertRaises(exceptions.SystemSetupError) as ctx:
supplier.create(cfg, sources=True, distribution=False, build=False, challenge_root_path="/", car=car, plugins=[
supplier.create(cfg, sources=True, distribution=False, build=False, car=car, plugins=[
core_plugin,
external_plugin
])
Expand All @@ -472,7 +472,7 @@ def test_create_suppliers_for_es_distribution_plugin_source_build(self):
external_plugin = team.PluginDescriptor("community-plugin", core_plugin=False)

# --revision="community-plugin:effab" --distribution-version="6.0.0"
composite_supplier = supplier.create(cfg, sources=False, distribution=True, build=False, challenge_root_path="/", car=car, plugins=[
composite_supplier = supplier.create(cfg, sources=False, distribution=True, build=False, car=car, plugins=[
core_plugin,
external_plugin
])
Expand Down Expand Up @@ -508,7 +508,7 @@ def test_create_suppliers_for_es_and_plugin_source_build(self):
external_plugin = team.PluginDescriptor("community-plugin", core_plugin=False)

# --revision="elasticsearch:abc,community-plugin:effab"
composite_supplier = supplier.create(cfg, sources=True, distribution=False, build=True, challenge_root_path="/", car=car, plugins=[
composite_supplier = supplier.create(cfg, sources=True, distribution=False, build=True, car=car, plugins=[
core_plugin,
external_plugin
])
Expand Down