jdaugherty commented on code in PR #16631:
URL: https://github.com/apache/grails-core/pull/16631#discussion_r4367377948


##########
grails-data-mongodb/core/src/main/groovy/org/apache/grails/data/mongo/core/querytail/MongoCommandRedactor.groovy:
##########
@@ -0,0 +1,237 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *    https://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+package org.apache.grails.data.mongo.core.querytail
+
+import groovy.transform.CompileStatic
+import org.bson.BsonDocument
+import org.bson.BsonValue
+
+import org.apache.grails.data.core.querytail.QueryTailRedactor
+
+/**
+ * Takes the values out of a MongoDB command before it is captured. A command 
is a document, so
+ * there is nothing to parse: every value becomes {@link 
QueryTailRedactor#PLACEHOLDER}, and the
+ * keys, which are the options of the command, its operators and the field 
names of the collection,
+ * are kept, except where a key may be data. The keys of a document stored in 
a map property, or in
+ * a property or a collection the {@linkplain MongoMappedFields mapping} does 
not know, are recorded
+ * as the placeholder too, and so is every segment of a dotted path past such 
a key. The entries the
+ * driver adds for its session, the cluster time and the transaction, which 
say nothing about the
+ * command, are left out.
+ *
+ * <p>A command is recorded as its name, the database and collection it runs 
against, and the
+ * document of its remaining entries: {@code find shop.book {"filter": 
{"title": ?}, "limit": ?}}.</p>
+ *
+ * @since 8.1
+ */
+@CompileStatic
+final class MongoCommandRedactor {
+
+    private static final String PLACEHOLDER = QueryTailRedactor.PLACEHOLDER
+
+    private static final String DATABASE = '$db'
+
+    private static final String COLLECTION = 'collection'
+
+    /** The entries the driver adds, which describe its session and 
transaction rather than the command. */
+    private static final Set<String> LEFT_OUT = ['lsid', '$clusterTime', 
'txnNumber', 'autocommit',
+            'startTransaction', '$readPreference', 'apiVersion', 'apiStrict', 
'apiDeprecationErrors'] as Set<String>
+
+    /**
+     * The options whose document, or documents, are keyed by the fields of 
the collection, wherever they appear,
+     * including those of the operations of a client-level {@code bulkWrite}.
+     */
+    private static final Set<String> FIELD_KEYED = ['filter', 'query', 'q', 
'u', 'update', 'document', 'documents',
+            'updateMods', 'pipeline', 'sort', 'projection', 'fields', 'key', 
'hint', 'min', 'max', 'arrayFilters',
+            'partialFilterExpression', 'wildcardProjection'] as Set<String>
+
+    private MongoCommandRedactor() {
+    }
+
+    /**
+     * Redacts a command.
+     *
+     * @param database the database the command runs against
+     * @param commandName the name of the command, which is its first key
+     * @param command the command as the driver sends it
+     * @param fields the fields the mapping of the datastore knows, or null 
before the datastore is built
+     * @return the command with its values replaced
+     */
+    static String redact(String database, String commandName, BsonDocument 
command, MongoMappedFields fields) {
+        String collection = null
+        String collectionKey = null
+        BsonValue first = command.get(commandName)
+        if (first != null && first.isString()) {
+            collection = first.asString().value
+            collectionKey = commandName
+        }
+        else if (command.get(COLLECTION)?.isString()) {
+            collection = command.getString(COLLECTION).value
+            collectionKey = COLLECTION
+        }
+        StringBuilder out = new StringBuilder(commandName)
+        if (database != null || collection != null) {
+            out.append(' ')
+            if (database != null) {
+                out.append(database)
+            }
+            if (collection != null) {
+                out.append(database != null ? '.' : '').append(collection)
+            }
+        }
+        Scope options = Scope.options(collection, fields)
+        StringBuilder body = new StringBuilder()
+        for (Map.Entry<String, BsonValue> entry in command.entrySet()) {
+            String key = entry.key
+            if (key == DATABASE || key == collectionKey || 
LEFT_OUT.contains(key)) {
+                continue
+            }
+            body.append(body.length() == 0 ? '{' : ', ')
+            appendEntry(body, key, entry.value, options)
+        }
+        if (body.length() > 0) {
+            out.append(' ').append(body).append('}')
+        }
+        out.toString()
+    }
+
+    private static void appendEntry(StringBuilder out, String key, BsonValue 
value, Scope scope) {
+        Scope child = scope.of(key)
+        out.append(child.keyText).append(': ')
+        appendValue(out, value, child)
+    }
+
+    private static void appendValue(StringBuilder out, BsonValue value, Scope 
scope) {
+        if (value.isDocument()) {
+            out.append('{')
+            boolean first = true
+            for (Map.Entry<String, BsonValue> entry in 
value.asDocument().entrySet()) {
+                if (!first) {
+                    out.append(', ')
+                }
+                first = false
+                appendEntry(out, entry.key, entry.value, scope)
+            }
+            out.append('}')
+        }
+        else if (value.isArray()) {
+            out.append('[')
+            boolean first = true
+            for (BsonValue element in value.asArray()) {
+                if (!first) {
+                    out.append(', ')
+                }
+                first = false
+                appendValue(out, element, scope)
+            }
+            out.append(']')
+        }
+        else {
+            out.append(PLACEHOLDER)
+        }
+    }
+
+    private static String quote(String key) {
+        StringBuilder out = new StringBuilder(key.length() + 2).append('"')
+        for (int i = 0; i < key.length(); i++) {
+            char c = key.charAt(i)
+            if (c == '"' || c == '\\') {
+                out.append('\\').append(c)
+            }
+            else if (c < 0x20) {
+                out.append(String.format('\\u%04x', (int) c))
+            }
+            else {
+                out.append(c)
+            }
+        }
+        out.append('"').toString()
+    }
+
+    /**
+     * Where a document sits in the command: among the options of the command, 
whose keys are kept
+     * and name what their documents hold, or in a document keyed by fields, 
where a key is kept when
+     * it is an operator or a field the mapping knows at its path.
+     */
+    @CompileStatic
+    private static final class Scope {
+
+        final String collection
+        final MongoMappedFields fields
+        /** The field path of the document; null in a document keyed by fields 
the mapping does not know. */
+        final List<String> path
+        final boolean optionKeyed
+        final String keyText
+
+        private Scope(String collection, MongoMappedFields fields, 
List<String> path, boolean optionKeyed,
+                String keyText) {
+            this.collection = collection
+            this.fields = fields
+            this.path = path
+            this.optionKeyed = optionKeyed
+            this.keyText = keyText
+        }
+
+        static Scope options(String collection, MongoMappedFields fields) {
+            new Scope(collection, fields, Collections.<String>emptyList(), 
true, null)
+        }
+
+        /**
+         * @return the scope of the value of a key in a document of this 
scope, with the key as it is recorded
+         */
+        Scope of(String key) {
+            if (optionKeyed) {
+                boolean fieldKeyed = FIELD_KEYED.contains(key)
+                return new Scope(collection, fields, 
Collections.<String>emptyList(), !fieldKeyed, quote(key))
+            }
+            if (key.startsWith('$')) {
+                return new Scope(collection, fields, path, false, quote(key))
+            }
+            List<String> current = path
+            String[] segments = key.split('\\.', -1)
+            StringBuilder recorded = new StringBuilder()
+            boolean anyKept = false
+            for (int i = 0; i < segments.length; i++) {
+                String segment = segments[i]
+                boolean kept
+                if (current == null) {
+                    kept = false
+                }
+                else if (MongoMappedFields.isIndex(segment)) {

Review Comment:
   Fixed in b3a97c2543 (folded into the feature commit). A numeric segment is 
now kept only when the path before it is a field the mapping knows as an array 
(`MongoMappedFields.isIndexed`: an array- or `Collection`-typed property, 
reached through embedded documents and collections as `isField` does); anywhere 
else, in a map property, under an unknown key, or at the top of a document, it 
is data and recorded as `?`, and the path past it is unknown. So `attrs: 
{"9876543210": "v"}` and `"attrs.9876543210"` both record `?`, while 
`stops.0.city` and `tags.0` keep the index. Covered in 
`MongoCommandRedactorSpec` (nested, dotted, top-level and `$set` forms, plus 
the never-survives case with a numeric secret) and through the capture API in 
`QueryTailCommandListenerSpec` against a real mongod, which saves a book whose 
`attrs` map has a numeric key, queries by that key, and asserts the key reaches 
neither the insert nor the find in the file. The guide says a numeric key is 
kept only where it i
 ndexes a list the mapping knows.



##########
grails-datamapping-core/src/main/groovy/org/apache/grails/data/core/querytail/QueryTailCapture.groovy:
##########
@@ -0,0 +1,356 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *    https://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+package org.apache.grails.data.core.querytail
+
+import java.nio.ByteBuffer
+import java.nio.channels.FileChannel
+import java.nio.channels.FileLock
+import java.nio.channels.OverlappingFileLockException
+import java.nio.charset.StandardCharsets
+import java.nio.file.Files
+import java.nio.file.StandardOpenOption
+import java.util.concurrent.atomic.AtomicBoolean
+
+import groovy.transform.CompileStatic
+import groovy.util.logging.Slf4j
+
+/**
+ * Writes the statements an application runs to the capture file the {@code 
query-tail} command
+ * tails. Each session factory, Neo4j driver or MongoDB client that captures 
opens its own capture
+ * and closes it when it closes; the captures of one file share it, so every 
data source of an
+ * application writes to the one file. The file is truncated when the first 
capture opens it, so it
+ * holds the statements of the current run only, and its first line names that 
run. It is closed
+ * when the last capture closes, and opened afresh by the next one. Each 
statement is written whole
+ * and at once, so that the command sees it as soon as it is recorded.
+ *
+ * <p>JVMs that run at the same time with capture enabled, such as the 
parallel forks of a test task,
+ * share the file: a lock file beside it tells the first to start, which 
truncates the file and names
+ * the run, from the others, which append to that run without truncating it 
and number their
+ * requests apart, so that the command reads one run from all of them.</p>
+ *
+ * <p>A statement is recorded without its values: {@link QueryTailRedactor} 
replaces its literals and
+ * drops its comments before it is written, in the {@link #syntax} of the 
database it runs on. A
+ * statement that is not text, such as a MongoDB command, has its values taken 
out by whoever records
+ * it, through {@link #recordRedacted}.</p>
+ *
+ * <p>A capture is only opened when {@link QueryTailSettings#SETTING_ENABLED} 
is set; an application
+ * started without it never reaches this class.</p>
+ *
+ * @since 8.1
+ */
+@CompileStatic
+@Slf4j
+class QueryTailCapture implements Closeable {
+
+    private static final Map<String, CaptureFile> OPEN = new HashMap<>()
+
+    /** The file the statements are written to. */
+    final File file
+
+    /** The syntax of the database the statements of this capture run on, 
which their values are found by. */
+    volatile QueryTailSyntax syntax = QueryTailSyntax.UNKNOWN
+
+    private final CaptureFile captureFile
+    private final AtomicBoolean closed = new AtomicBoolean()
+
+    private QueryTailCapture(File file, CaptureFile captureFile) {
+        this.file = file
+        this.captureFile = captureFile
+    }
+
+    /**
+     * Opens a capture of a file. The first capture of a file truncates it; a 
capture opened while
+     * another of the same file is open shares it. Every capture opened must 
be closed.
+     *
+     * @param file the capture file
+     * @return the capture
+     * @throws UncheckedIOException when the file cannot be created
+     */
+    static QueryTailCapture open(File file) {
+        String key
+        try {
+            key = file.canonicalPath
+        }
+        catch (IOException e) {
+            throw new UncheckedIOException("Cannot resolve the query-tail 
capture file ${file}: ${e.message}", e)
+        }
+        synchronized (OPEN) {
+            CaptureFile captureFile = OPEN.get(key)
+            if (captureFile == null) {
+                captureFile = new CaptureFile(file, key)
+                OPEN.put(key, captureFile)
+                if (captureFile.joined) {
+                    log.info('Capturing statements to {} for query-tail, in 
the run of another JVM writing it', file)
+                }
+                else {
+                    log.info('Capturing statements to {} for query-tail', file)
+                }
+            }
+            captureFile.captures++
+            return new QueryTailCapture(file, captureFile)
+        }
+    }
+
+    /**
+     * Opens the capture the settings name.
+     *
+     * @param settings the query-tail settings
+     * @return the capture
+     */
+    static QueryTailCapture open(QueryTailSettings settings) {
+        open(settings.resolveFile())
+    }
+
+    /**
+     * Records a statement that is about to run on the current thread, without 
its values.
+     *
+     * @param dataSource the connection source name the statement runs against
+     * @param sql the statement
+     */
+    void record(String dataSource, String sql) {
+        write(dataSource, QueryTailRedactor.redact(sql, syntax))
+    }
+
+    /**
+     * Records a statement that is about to run on the current thread and that 
holds no value any
+     * more: a MongoDB command, which is a document rather than text, is 
redacted field by field by
+     * the listener that records it. Nothing of the statement is changed here.
+     *
+     * @param dataSource the connection source name the statement runs against
+     * @param statement the statement, with its values already taken out
+     */
+    void recordRedacted(String dataSource, String statement) {
+        write(dataSource, statement)
+    }
+
+    private void write(String dataSource, String statement) {
+        if (closed.get()) {
+            return
+        }
+        QueryTailRecord record = new QueryTailRecord(timestamp: 
System.currentTimeMillis(), dataSource: dataSource,
+                sql: statement)
+        captureFile.write(captureFile.attribution.attribute(record))
+    }
+
+    /**
+     * @return whether this capture is still writing to its file
+     */
+    boolean isOpen() {
+        !closed.get() && captureFile.open
+    }
+
+    /**
+     * Closes this capture. The file is closed once every capture of it is.
+     */
+    @Override
+    void close() {
+        if (!closed.compareAndSet(false, true)) {
+            return
+        }
+        boolean last
+        synchronized (OPEN) {
+            last = --captureFile.captures == 0
+            if (last && OPEN.get(captureFile.key).is(captureFile)) {
+                OPEN.remove(captureFile.key)
+            }
+        }
+        if (last) {
+            captureFile.close()
+        }
+    }
+
+    /**
+     * A capture file and the captures writing to it. The file is written by 
one channel, a line at a
+     * time, and the requests of every data source are numbered by one 
attribution, so that two
+     * requests never share an id.
+     *
+     * <p>A lock file beside the capture coordinates the JVMs writing it. 
Every writer holds a shared
+     * lock on the {@link #WRITING} byte of the lock file while it writes. A 
starting writer first
+     * takes the {@link #STARTING} byte exclusively, so that starters take 
turns, then tries the
+     * {@link #WRITING} byte exclusively: when it gets it, no other JVM is 
writing, and it truncates the
+     * file and writes the header of a new run; when it does not, it joins the 
run being written. Either
+     * way it then holds the {@link #WRITING} byte shared until it closes. A 
lock file that cannot be
+     * created, as beside a device, leaves the writer uncoordinated, as the 
only one.</p>
+     */
+    @CompileStatic
+    private static class CaptureFile {
+
+        /** The byte of the lock file a starting writer holds, exclusively, 
while it decides whether to truncate. */
+        private static final long STARTING = 0L
+
+        /** The byte of the lock file every writer holds, shared, while it 
writes. */
+        private static final long WRITING = 1L
+
+        final File file
+        final String key
+        final QueryTailRequestAttribution attribution
+
+        /** Whether this JVM joined the run of another JVM writing the file, 
rather than starting one. */
+        final boolean joined
+
+        /** How many captures are open on the file; guarded by {@link 
QueryTailCapture#OPEN}. */
+        int captures
+
+        private FileChannel channel
+        private FileChannel lockChannel
+        private FileLock writing
+
+        CaptureFile(File file, String key) {
+            this.file = file
+            this.key = key
+            File parent = file.absoluteFile.parentFile
+            boolean first
+            try {
+                if (parent != null && !parent.isDirectory() && 
!parent.mkdirs() && !parent.isDirectory()) {
+                    throw new IOException("Cannot create the query-tail 
capture directory ${parent}")
+                }
+                lockChannel = openLockFile(file)
+                FileLock starting = lockChannel == null ? null : 
lockChannel.lock(STARTING, 1L, false)
+                try {
+                    FileLock alone = lockChannel == null ? null : 
lock(lockChannel, false, true)
+                    first = lockChannel == null || alone != null
+                    alone?.release()
+                    if (first) {
+                        Files.newByteChannel(file.toPath(), 
StandardOpenOption.CREATE, StandardOpenOption.WRITE,
+                                StandardOpenOption.TRUNCATE_EXISTING).close()
+                    }
+                    channel = FileChannel.open(file.toPath(), 
StandardOpenOption.CREATE, StandardOpenOption.WRITE,
+                            StandardOpenOption.APPEND)
+                    writing = lockChannel == null ? null : lock(lockChannel, 
true, false)
+                }
+                finally {

Review Comment:
   Fixed in b3a97c2543. The starter now writes the run header while it still 
holds the `STARTING` byte: the header write moved inside the locked section, 
after the shared `WRITING` lock is taken, so a second JVM that is blocked on 
`STARTING` cannot append anything before the header, and the javadoc of 
`CaptureFile` describes the order. I did not find a way to drive that window 
from a single JVM, since a second lock on the same byte in one process throws 
`OverlappingFileLockException` instead of blocking, so there is no new spec for 
the ordering itself; the existing joined-run and truncation specs still pass.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to