From 07826ae7022a22f5d62da90a1a1bc0db5e78b33a Mon Sep 17 00:00:00 2001 From: Emma Holmberg Ohlsson Date: Fri, 19 Jan 2024 09:50:48 +0100 Subject: [PATCH] Report command handles several dbs and allows for specifying which By default report will collect for all databases --- .../dbms/DiagnosticsReportCommandIT.java | 49 ++++++------ .../dbms/DiagnosticsReportCommand.java | 80 +++++++++++++++---- .../dbms/DiagnosticsReportCommandTest.java | 10 ++- .../DiagnosticsOfflineReportProvider.java | 13 ++- .../diagnostics/DiagnosticsReporter.java | 4 +- ...ernelDiagnosticsOfflineReportProvider.java | 30 ++++--- .../diagnostics/DiagnosticsReporterTest.java | 2 +- .../server/startup/Neo4jAdminCommandTest.java | 10 ++- ...erverDiagnosticsOfflineReportProvider.java | 2 +- 9 files changed, 135 insertions(+), 65 deletions(-) diff --git a/community/community-it/dbms-it/src/test/java/org/neo4j/commandline/dbms/DiagnosticsReportCommandIT.java b/community/community-it/dbms-it/src/test/java/org/neo4j/commandline/dbms/DiagnosticsReportCommandIT.java index 53ba8cf1c24b8..4d4785c8ad8be 100644 --- a/community/community-it/dbms-it/src/test/java/org/neo4j/commandline/dbms/DiagnosticsReportCommandIT.java +++ b/community/community-it/dbms-it/src/test/java/org/neo4j/commandline/dbms/DiagnosticsReportCommandIT.java @@ -19,6 +19,7 @@ */ package org.neo4j.commandline.dbms; +import java.nio.file.StandardOpenOption; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -75,6 +76,7 @@ class DiagnosticsReportCommandIT void setUp() throws Exception { homeDir = testDirectory.directory( "home-dir" ); + createDatabaseDir(homeDir); configDir = testDirectory.directory( "config-dir" ); // Touch config @@ -87,6 +89,12 @@ void setUp() throws Exception ctx = new ExecutionContext( homeDir, configDir, System.out, System.err, fs ); } + private void createDatabaseDir( Path homeDir ) throws IOException + { + // Database directory needed for command to be able to collect anything + Files.createDirectories( homeDir.resolve( "data" ).resolve( "databases" ).resolve( "neo4j" )); + } + @AfterEach void tearDown() { @@ -100,19 +108,15 @@ void shouldBeAbleToAttachToPidAndRunThreadDump() throws IOException long pid = getPID(); assertThat( pid ).isNotEqualTo( 0 ); - // Write config file - Files.createFile( testDirectory.file( "neo4j.conf" ) ); - // write neo4j.pid file - Path run = testDirectory.directory( "run" ); + Path run = testDirectory.directory( "run", homeDir.getFileName().toString()); Files.write( run.resolve( "neo4j.pid" ), String.valueOf( pid ).getBytes() ); // Run command, should detect running instance try { String[] args = {"threads", "--to=" + testDirectory.absolutePath() + "/reports"}; - Path homeDir = testDirectory.homePath(); - var ctx = new ExecutionContext( homeDir, homeDir, System.out, System.err, testDirectory.getFileSystem() ); + var ctx = new ExecutionContext( homeDir, configDir, System.out, System.err, testDirectory.getFileSystem() ); DiagnosticsReportCommand diagnosticsReportCommand = new DiagnosticsReportCommand( ctx ); CommandLine.populateCommand( diagnosticsReportCommand, args ); diagnosticsReportCommand.execute(); @@ -148,19 +152,15 @@ void shouldBeAbleToAttachToPidAndRunHeapDump() throws IOException long pid = getPID(); assertThat( pid ).isNotEqualTo( 0 ); - // Write config file - Files.createFile( testDirectory.file( "neo4j.conf" ) ); - // write neo4j.pid file - Path run = testDirectory.directory( "run" ); - Files.write( run.resolve( "neo4j.pid" ), String.valueOf( pid ).getBytes() ); + Path run = testDirectory.directory( "run", homeDir.getFileName().toString() ); + Files.write( run.resolve( "neo4j.pid" ), String.valueOf( pid ).getBytes(), StandardOpenOption.CREATE ); // Run command, should detect running instance try { String[] args = {"heap", "--to=" + testDirectory.absolutePath() + "/reports"}; - Path homeDir = testDirectory.homePath(); - var ctx = new ExecutionContext( homeDir, homeDir, System.out, System.err, testDirectory.getFileSystem() ); + var ctx = new ExecutionContext( homeDir, configDir, System.out, System.err, testDirectory.getFileSystem() ); DiagnosticsReportCommand diagnosticsReportCommand = new DiagnosticsReportCommand( ctx ); CommandLine.populateCommand( diagnosticsReportCommand, args ); diagnosticsReportCommand.execute(); @@ -190,19 +190,20 @@ void shouldBeAbleToAttachToPidAndRunHeapDump() throws IOException void shouldHandleRotatedLogFiles() throws IOException { // Write config file and specify a custom name for the neo4j.log file. - Path confFile = testDirectory.createFile( "neo4j.conf" ); - Files.write( confFile, singletonList( GraphDatabaseSettings.store_user_log_path.name() + "=custom.neo4j.log.name" ) ); - - // Create some log files that should be found. debug.log has already been created during setup. - testDirectory.directory( "logs" ); - testDirectory.createFile( "logs/debug.log" ); - testDirectory.createFile( "logs/debug.log.1.zip" ); - testDirectory.createFile( "logs/custom.neo4j.log.name" ); - testDirectory.createFile( "logs/custom.neo4j.log.name.1" ); + Files.write( + configDir.resolve("neo4j.conf"), singletonList(GraphDatabaseSettings.store_user_log_path.name() + "=custom.neo4j.log.name")); + + // Create some log files that should be found. + Path logDir = + testDirectory.directory( "logs", homeDir.getFileName().toString() ); + FileSystemAbstraction fs = testDirectory.getFileSystem(); + fs.write( logDir.resolve( "debug.log" ) ); + fs.write( logDir.resolve( "debug.log.1.zip" ) ); + fs.write( logDir.resolve( "custom.neo4j.log.name" ) ); + fs.write( logDir.resolve( "custom.neo4j.log.name.1" ) ); String[] args = {"logs", "--to=" + testDirectory.absolutePath() + "/reports"}; - Path homeDir = testDirectory.homePath(); - var ctx = new ExecutionContext( homeDir, homeDir, System.out, System.err, testDirectory.getFileSystem() ); + var ctx = new ExecutionContext( homeDir, configDir, System.out, System.err, testDirectory.getFileSystem() ); DiagnosticsReportCommand diagnosticsReportCommand = new DiagnosticsReportCommand( ctx ); CommandLine.populateCommand( diagnosticsReportCommand, args ); diagnosticsReportCommand.execute(); diff --git a/community/dbms/src/main/java/org/neo4j/commandline/dbms/DiagnosticsReportCommand.java b/community/dbms/src/main/java/org/neo4j/commandline/dbms/DiagnosticsReportCommand.java index 5e9274284c86f..11d70ee7f78f9 100644 --- a/community/dbms/src/main/java/org/neo4j/commandline/dbms/DiagnosticsReportCommand.java +++ b/community/dbms/src/main/java/org/neo4j/commandline/dbms/DiagnosticsReportCommand.java @@ -19,8 +19,12 @@ */ package org.neo4j.commandline.dbms; -import org.jutils.jprocesses.JProcesses; -import org.jutils.jprocesses.model.ProcessInfo; +import static java.lang.String.join; +import static org.apache.commons.text.StringEscapeUtils.escapeCsv; +import static picocli.CommandLine.Command; +import static picocli.CommandLine.Help.Visibility.NEVER; +import static picocli.CommandLine.Option; +import static picocli.CommandLine.Parameters; import java.io.IOException; import java.net.InetAddress; @@ -33,15 +37,21 @@ import java.util.Optional; import java.util.Set; import java.util.TreeSet; - +import org.eclipse.collections.impl.factory.Sets; +import org.jutils.jprocesses.JProcesses; +import org.jutils.jprocesses.model.ProcessInfo; import org.neo4j.cli.AbstractCommand; import org.neo4j.cli.CommandFailedException; +import org.neo4j.cli.Converters; import org.neo4j.cli.ExecutionContext; import org.neo4j.configuration.Config; import org.neo4j.configuration.ConfigUtils; import org.neo4j.configuration.GraphDatabaseSettings; +import org.neo4j.configuration.helpers.DatabaseNamePattern; import org.neo4j.dbms.diagnostics.jmx.JMXDumper; import org.neo4j.dbms.diagnostics.jmx.JmxDump; +import org.neo4j.io.fs.FileSystemAbstraction; +import org.neo4j.io.layout.Neo4jLayout; import org.neo4j.kernel.diagnostics.DiagnosticsReportSource; import org.neo4j.kernel.diagnostics.DiagnosticsReportSources; import org.neo4j.kernel.diagnostics.DiagnosticsReporter; @@ -49,14 +59,6 @@ import org.neo4j.kernel.diagnostics.InteractiveProgress; import org.neo4j.kernel.diagnostics.NonInteractiveProgress; -import static java.lang.String.join; -import static org.apache.commons.text.StringEscapeUtils.escapeCsv; -import static org.neo4j.configuration.GraphDatabaseInternalSettings.databases_root_path; -import static picocli.CommandLine.Command; -import static picocli.CommandLine.Help.Visibility.NEVER; -import static picocli.CommandLine.Option; -import static picocli.CommandLine.Parameters; - @Command( name = "report", header = "Produces a zip/tar of the most common information needed for remote assessments.", @@ -70,6 +72,16 @@ public class DiagnosticsReportCommand extends AbstractCommand private static final DateTimeFormatter filenameDateTimeFormatter = DateTimeFormatter.ofPattern("yyyy-MM-dd_HHmmss" ); private static final long NO_PID = 0; + @Option( + names = "--database", + paramLabel = "", + defaultValue = "*", + description = "Name of the database to report for. Can contain * and ? for globbing. " + + "Note that * and ? have special meaning in some shells " + + "and might need to be escaped or used with quotes.", + converter = Converters.DatabaseNamePatternConverter.class ) + private DatabaseNamePattern database; + @Option( names = "--list", arity = "0", description = "List all available classifiers" ) private boolean list; @@ -98,7 +110,8 @@ public void execute() Config config = getConfig( configFile() ); jmxDumper = new JMXDumper( config, ctx.fs(), ctx.out(), ctx.err(), verbose ); - DiagnosticsReporter reporter = createAndRegisterSources( config ); + Set dbNames = getDbNames(config, ctx.fs(), database); + DiagnosticsReporter reporter = createAndRegisterSources(config, dbNames); if ( list ) { @@ -182,13 +195,11 @@ private void listClassifiers( Set availableClassifiers ) } } - private DiagnosticsReporter createAndRegisterSources( Config config ) + private DiagnosticsReporter createAndRegisterSources( Config config, Set databaseNames ) { DiagnosticsReporter reporter = new DiagnosticsReporter(); - Path storeDirectory = config.get( databases_root_path ); - - reporter.registerAllOfflineProviders( config, storeDirectory, ctx.fs(), config.get( GraphDatabaseSettings.default_database ) ); + reporter.registerAllOfflineProviders( config, ctx.fs(), databaseNames ); // Register sources provided by this tool reporter.registerSource( "config", @@ -309,4 +320,41 @@ private static DiagnosticsReportSource runningProcesses() return sb.toString(); } ); } + + private Set getDbNames( Config config, FileSystemAbstraction fs, DatabaseNamePattern database ) + { + if ( !database.containsPattern() ) + { + return Set.of( database.getDatabaseName() ); + } + else + { + Set dbNames = Sets.mutable.empty(); + Path databasesDir = Neo4jLayout.of( config ).databasesDirectory(); + try + { + for ( Path path : fs.listFiles( databasesDir ) ) + { + if ( fs.isDirectory( path ) ) + { + String name = path.getFileName().toString(); + if ( database.matches( name ) ) + { + dbNames.add( name ); + } + } + } + } + catch ( IOException e ) + { + throw new CommandFailedException( "Failed to list databases", e ); + } + if ( dbNames.isEmpty() ) + { + throw new CommandFailedException( + "Pattern '" + database.getDatabaseName() + "' did not match any database" ); + } + return dbNames; + } + } } diff --git a/community/dbms/src/test/java/org/neo4j/commandline/dbms/DiagnosticsReportCommandTest.java b/community/dbms/src/test/java/org/neo4j/commandline/dbms/DiagnosticsReportCommandTest.java index 1efbe7d4a8f26..ad1a107cd8960 100644 --- a/community/dbms/src/test/java/org/neo4j/commandline/dbms/DiagnosticsReportCommandTest.java +++ b/community/dbms/src/test/java/org/neo4j/commandline/dbms/DiagnosticsReportCommandTest.java @@ -46,8 +46,8 @@ void printUsageHelp() "%n" + "USAGE%n" + "%n" + - "report [--expand-commands] [--force] [--list] [--verbose] [--pid=]%n" + - " [--to=] [...]%n" + + "report [--expand-commands] [--force] [--list] [--verbose]%n" + + " [--database=] [--pid=] [--to=] [...]%n" + "%n" + "DESCRIPTION%n" + "%n" + @@ -64,6 +64,12 @@ void printUsageHelp() "%n" + " --verbose Enable verbose output.%n" + " --expand-commands Allow command expansion in config value evaluation.%n" + + " --database=%n" + + " Name of the database to report for. Can contain * and%n" + + " ? for globbing. Note that * and ? have special%n" + + " meaning in some shells and might need to be escaped%n" + + " or used with quotes.%n" + + " Default: *%n" + " --list List all available classifiers%n" + " --force Ignore disk full warning%n" + " --to= Destination directory for reports. Defaults to a%n" + diff --git a/community/kernel/src/main/java/org/neo4j/kernel/diagnostics/DiagnosticsOfflineReportProvider.java b/community/kernel/src/main/java/org/neo4j/kernel/diagnostics/DiagnosticsOfflineReportProvider.java index f18b2c00261dc..5799f37d33261 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/diagnostics/DiagnosticsOfflineReportProvider.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/diagnostics/DiagnosticsOfflineReportProvider.java @@ -19,7 +19,6 @@ */ package org.neo4j.kernel.diagnostics; -import java.nio.file.Path; import java.util.Arrays; import java.util.HashSet; import java.util.List; @@ -33,7 +32,7 @@ * Base class for a provider of offline reports. Offline reports does not require a running instance of the database * and is intended to be use as a way to gather information even if the database cannot be started. All implementing * classes is service loaded and initialized through the - * {@link DiagnosticsOfflineReportProvider#init(FileSystemAbstraction, String, Config, Path)} method. + * {@link DiagnosticsOfflineReportProvider#init(FileSystemAbstraction, Config, Set)} method. */ @Service public abstract class DiagnosticsOfflineReportProvider @@ -55,12 +54,12 @@ protected DiagnosticsOfflineReportProvider( String classifier, String... classif /** * Called after service loading to initialize the class. - * @param fs filesystem to use for file access. - * @param defaultDatabaseName identifier for default database - * @param config configuration file in use. - * @param storeDirectory directory of the database files. + * + * @param fs filesystem to use for file access. + * @param config configuration file in use. + * @param databaseNames the databases to report for. */ - public abstract void init( FileSystemAbstraction fs, String defaultDatabaseName, Config config, Path storeDirectory ); + public abstract void init( FileSystemAbstraction fs, Config config, Set databaseNames ); /** * Returns a list of source that matches the given classifiers. diff --git a/community/kernel/src/main/java/org/neo4j/kernel/diagnostics/DiagnosticsReporter.java b/community/kernel/src/main/java/org/neo4j/kernel/diagnostics/DiagnosticsReporter.java index 81a47ab39dbbc..2bc57904ad67f 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/diagnostics/DiagnosticsReporter.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/diagnostics/DiagnosticsReporter.java @@ -143,11 +143,11 @@ public Set getAvailableClassifiers() return availableClassifiers; } - public void registerAllOfflineProviders( Config config, Path storeDirectory, FileSystemAbstraction fs, String defaultDatabaseName ) + public void registerAllOfflineProviders( Config config, FileSystemAbstraction fs, Set databaseNames ) { for ( DiagnosticsOfflineReportProvider provider : Services.loadAll( DiagnosticsOfflineReportProvider.class ) ) { - provider.init( fs, defaultDatabaseName, config, storeDirectory ); + provider.init( fs, config, databaseNames ); registerOfflineProvider( provider ); } } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/diagnostics/KernelDiagnosticsOfflineReportProvider.java b/community/kernel/src/main/java/org/neo4j/kernel/diagnostics/KernelDiagnosticsOfflineReportProvider.java index bdb9664c15942..05f3f3311d068 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/diagnostics/KernelDiagnosticsOfflineReportProvider.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/diagnostics/KernelDiagnosticsOfflineReportProvider.java @@ -43,7 +43,8 @@ public class KernelDiagnosticsOfflineReportProvider extends DiagnosticsOfflineRe { private FileSystemAbstraction fs; private Config config; - private DatabaseLayout databaseLayout; + private Set databaseNames; + private Neo4jLayout neo4jLayout; public KernelDiagnosticsOfflineReportProvider() { @@ -51,11 +52,12 @@ public KernelDiagnosticsOfflineReportProvider() } @Override - public void init( FileSystemAbstraction fs, String defaultDatabaseName, Config config, Path storeDirectory ) + public void init( FileSystemAbstraction fs, Config config, Set databaseNames ) { this.fs = fs; this.config = config; - this.databaseLayout = DatabaseLayout.of( Neo4jLayout.of(config), defaultDatabaseName ); + this.databaseNames = databaseNames; + this.neo4jLayout = Neo4jLayout.of( config ); } @Override @@ -72,11 +74,17 @@ protected List provideSources( Set classifiers } if ( classifiers.contains( "tree" ) ) { - listDataDirectory( sources ); + for ( String databaseName : databaseNames ) + { + listDataDirectory(sources, databaseName); + } } if ( classifiers.contains( "tx" ) ) { - getTransactionLogFiles( sources ); + for ( String databaseName : databaseNames ) + { + getTransactionLogFiles(sources, databaseName); + } } if ( classifiers.contains( "version" ) ) { @@ -143,15 +151,16 @@ private void listContentOfDirectory( Path directory, String prefix, StringBuilde * * @param sources destination of the sources. */ - private void listDataDirectory( List sources ) + private void listDataDirectory( List sources, String databaseName ) { + DatabaseLayout databaseLayout = DatabaseLayout.of(neo4jLayout, databaseName); StorageEngineFactory storageEngineFactory = StorageEngineFactory.defaultStorageEngine(); StoreFilesDiagnostics storeFiles = new StoreFilesDiagnostics( storageEngineFactory, fs, databaseLayout ); List files = new ArrayList<>(); storeFiles.dump( files::add ); - sources.add( DiagnosticsReportSources.newDiagnosticsString( "tree.txt", () -> String.join( System.lineSeparator(), files ) ) ); + sources.add( DiagnosticsReportSources.newDiagnosticsString( databaseName + "/tree.txt", () -> String.join( System.lineSeparator(), files ) ) ); } /** @@ -190,20 +199,21 @@ private void getLogFiles( List sources ) * * @param sources destination of the sources. */ - private void getTransactionLogFiles( List sources ) + private void getTransactionLogFiles( List sources, String databaseName ) { try { + DatabaseLayout databaseLayout = DatabaseLayout.of(neo4jLayout, databaseName); LogFiles logFiles = LogFilesBuilder.logFilesBasedOnlyBuilder( databaseLayout.getTransactionLogsDirectory(), fs ).build(); for ( Path file : logFiles.logFiles() ) { - sources.add( DiagnosticsReportSources.newDiagnosticsFile( "tx/" + file.getFileName(), fs, file ) ); + sources.add( DiagnosticsReportSources.newDiagnosticsFile( databaseName + "/tx/" + file.getFileName(), fs, file ) ); } } catch ( IOException e ) { sources.add( DiagnosticsReportSources - .newDiagnosticsString( "tx.txt", () -> "Error getting tx logs: " + e.getMessage() ) ); + .newDiagnosticsString( databaseName + "/tx.txt", () -> "Error getting tx logs: " + e.getMessage() ) ); } } } diff --git a/community/kernel/src/test/java/org/neo4j/kernel/diagnostics/DiagnosticsReporterTest.java b/community/kernel/src/test/java/org/neo4j/kernel/diagnostics/DiagnosticsReporterTest.java index 00034f40e6b0b..44bd4d6caad27 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/diagnostics/DiagnosticsReporterTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/diagnostics/DiagnosticsReporterTest.java @@ -175,7 +175,7 @@ void addFile( String destination, Path file ) } @Override - public void init( FileSystemAbstraction fs, String defaultDatabaseName, Config config, Path storeDirectory ) + public void init( FileSystemAbstraction fs, Config config, Set databaseNames ) { } diff --git a/community/neo4j/src/test/java/org/neo4j/server/startup/Neo4jAdminCommandTest.java b/community/neo4j/src/test/java/org/neo4j/server/startup/Neo4jAdminCommandTest.java index d53df18580d01..22008ff4fffdb 100644 --- a/community/neo4j/src/test/java/org/neo4j/server/startup/Neo4jAdminCommandTest.java +++ b/community/neo4j/src/test/java/org/neo4j/server/startup/Neo4jAdminCommandTest.java @@ -125,10 +125,16 @@ void shouldNotExpandAtFilesInBootloader() throws Exception @DisabledOnOs( OS.WINDOWS ) void shouldPassThroughAndAcceptVerboseAndExpandCommands() throws Exception { - addConf( GraphDatabaseSettings.default_database, "$(echo foo)" ); + // The command will fail if the databases directory doesn't exist. + // Exception would be thrown if expand commands didn't work. + Path customDbDir = home.resolve("customDbDir"); + Path databasesDir = customDbDir.resolve("databases"); + Files.createDirectories(databasesDir.resolve("customDbName")); + + addConf(GraphDatabaseSettings.data_directory, "$(echo " + customDbDir.toAbsolutePath() + ")"); if ( fork.run( () -> assertThat( execute( "report", "--to", home.toString(), "--verbose", "--expand-commands" ) ).isEqualTo( EXIT_CODE_OK ) ) ) { - assertThat( out.toString() ).containsSubsequence( "Writing report to", "100%" ); + assertThat(out.toString()).containsSubsequence("Writing report to", "customDbName", "100%"); assertThat( err.toString() ).isEmpty(); } } diff --git a/community/server/src/main/java/org/neo4j/server/diagnostics/ServerDiagnosticsOfflineReportProvider.java b/community/server/src/main/java/org/neo4j/server/diagnostics/ServerDiagnosticsOfflineReportProvider.java index 33fc8b992be2b..316bb90b37f5d 100644 --- a/community/server/src/main/java/org/neo4j/server/diagnostics/ServerDiagnosticsOfflineReportProvider.java +++ b/community/server/src/main/java/org/neo4j/server/diagnostics/ServerDiagnosticsOfflineReportProvider.java @@ -45,7 +45,7 @@ public ServerDiagnosticsOfflineReportProvider() } @Override - public void init( FileSystemAbstraction fs, String defaultDatabaseName, Config config, Path storeDirectory ) + public void init( FileSystemAbstraction fs, Config config, Set databaseNames ) { this.fs = fs; this.config = config;