From 7b9f05860e4906dc68a4bbd5d8382976eb720d74 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Mon, 28 Oct 2019 14:43:39 +0100 Subject: Trace sources of query profile values --- .../src/main/java/com/yahoo/search/Query.java | 10 +++--- .../profile/AllValuesQueryProfileVisitor.java | 2 +- .../profile/BackedOverridableQueryProfile.java | 21 ++++++------ .../query/profile/OverridableQueryProfile.java | 8 ++--- .../yahoo/search/query/profile/QueryProfile.java | 38 +++++++++++++--------- .../profile/compiled/CompiledQueryProfile.java | 21 ++++++++++++ .../query/profile/compiled/ValueWithSource.java | 5 ++- .../profile/config/QueryProfileConfigurer.java | 2 +- .../profile/config/QueryProfileXMLReader.java | 2 +- .../query/profile/test/QueryProfileTestCase.java | 2 +- .../search/rendering/JsonRendererTestCase.java | 4 +-- .../java/com/yahoo/search/test/QueryTestCase.java | 23 ++++++------- 12 files changed, 84 insertions(+), 54 deletions(-) (limited to 'container-search') diff --git a/container-search/src/main/java/com/yahoo/search/Query.java b/container-search/src/main/java/com/yahoo/search/Query.java index 84827a46572..3dabf9bc649 100644 --- a/container-search/src/main/java/com/yahoo/search/Query.java +++ b/container-search/src/main/java/com/yahoo/search/Query.java @@ -482,14 +482,14 @@ public class Query extends com.yahoo.processing.Request implements Cloneable { resolvedValue = requestProperty.getValue(); b.append(requestProperty.getKey()); - b.append("="); + b.append(": "); b.append(resolvedValue); // (may be null) b.append(" ("); if (profile != null && ! profile.isOverridable(new CompoundName(requestProperty.getKey()), requestProperties())) - b.append("value from query profile - unoverridable, ignoring request value"); + b.append("from query profile - unoverridable, ignoring request value"); else - b.append("value from request"); + b.append("from request"); b.append(")\n"); mentioned.add(requestProperty.getKey()); } @@ -504,9 +504,9 @@ public class Query extends com.yahoo.processing.Request implements Cloneable { } private void appendQueryProfileProperties(CompiledQueryProfile profile, Set mentioned, StringBuilder b) { - for (Map.Entry property : profile.listValues(CompoundName.empty, requestProperties(), properties()).entrySet()) { + for (var property : profile.listValuesWithSources(CompoundName.empty, requestProperties(), properties()).entrySet()) { if ( ! mentioned.contains(property.getKey())) - b.append(property.getKey()).append("=").append(property.getValue()).append(" (value from query profile)\n"); + b.append(property.getKey()).append(": ").append(property.getValue()).append("\n"); } } diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/AllValuesQueryProfileVisitor.java b/container-search/src/main/java/com/yahoo/search/query/profile/AllValuesQueryProfileVisitor.java index 93025a2d68c..6886b53e1d4 100644 --- a/container-search/src/main/java/com/yahoo/search/query/profile/AllValuesQueryProfileVisitor.java +++ b/container-search/src/main/java/com/yahoo/search/query/profile/AllValuesQueryProfileVisitor.java @@ -43,7 +43,7 @@ final class AllValuesQueryProfileVisitor extends PrefixQueryProfileVisitor { if (fullName.isEmpty()) return; // Avoid putting a non-leaf (subtree) root in the list if (values.containsKey(fullName.toString())) return; // The first value encountered has priority - values.put(fullName.toString(), new ValueWithSource(value, owner.getIdString(), variant)); + values.put(fullName.toString(), new ValueWithSource(value, owner.getSource(), variant)); } /** Returns the values resulting from this visiting */ diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/BackedOverridableQueryProfile.java b/container-search/src/main/java/com/yahoo/search/query/profile/BackedOverridableQueryProfile.java index 8c720b516a9..e6af1311bc3 100644 --- a/container-search/src/main/java/com/yahoo/search/query/profile/BackedOverridableQueryProfile.java +++ b/container-search/src/main/java/com/yahoo/search/query/profile/BackedOverridableQueryProfile.java @@ -44,21 +44,23 @@ public class BackedOverridableQueryProfile extends OverridableQueryProfile imple @Override protected Object localLookup(String localName, DimensionBinding dimensionBinding) { - Object valueInThis=super.localLookup(localName,dimensionBinding); - if (valueInThis!=null) return valueInThis; - return backingProfile.localLookup(localName,dimensionBinding); + Object valueInThis = super.localLookup(localName, dimensionBinding); + if (valueInThis != null) return valueInThis; + return backingProfile.localLookup(localName, dimensionBinding); } protected Boolean isLocalInstanceOverridable(String localName) { - Boolean valueInThis=super.isLocalInstanceOverridable(localName); - if (valueInThis!=null) return valueInThis; + Boolean valueInThis = super.isLocalInstanceOverridable(localName); + if (valueInThis != null) return valueInThis; return backingProfile.isLocalInstanceOverridable(localName); } @Override - protected QueryProfile createSubProfile(String name,DimensionBinding dimensionBinding) { - Object backing=backingProfile.lookup(new CompoundName(name),true,dimensionBinding.createFor(backingProfile.getDimensions())); - if (backing!=null && backing instanceof QueryProfile) + protected QueryProfile createSubProfile(String name, DimensionBinding dimensionBinding) { + Object backing = backingProfile.lookup(new CompoundName(name), + true, + dimensionBinding.createFor(backingProfile.getDimensions())); + if (backing instanceof QueryProfile) return new BackedOverridableQueryProfile((QueryProfile)backing); else return new OverridableQueryProfile(); // Nothing is set in this branch, so nothing to override, but need override checking @@ -67,8 +69,7 @@ public class BackedOverridableQueryProfile extends OverridableQueryProfile imple /** Returns a clone of this which can be independently overridden, but which refers to the same backing profile */ @Override public BackedOverridableQueryProfile clone() { - BackedOverridableQueryProfile clone=(BackedOverridableQueryProfile)super.clone(); - return clone; + return (BackedOverridableQueryProfile)super.clone(); } /** Returns the query profile backing this */ diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/OverridableQueryProfile.java b/container-search/src/main/java/com/yahoo/search/query/profile/OverridableQueryProfile.java index a275e3697a2..325c85ac491 100644 --- a/container-search/src/main/java/com/yahoo/search/query/profile/OverridableQueryProfile.java +++ b/container-search/src/main/java/com/yahoo/search/query/profile/OverridableQueryProfile.java @@ -22,15 +22,15 @@ public class OverridableQueryProfile extends QueryProfile { @Override protected Object checkAndConvertAssignment(String localName, Object inputValue, QueryProfileRegistry registry) { - Object value=super.checkAndConvertAssignment(localName, inputValue, registry); - if (value!=null && value.getClass() == QueryProfile.class) { // We are assigning a query profile - make it overridable + Object value = super.checkAndConvertAssignment(localName, inputValue, registry); + if (value != null && value.getClass() == QueryProfile.class) { // We are assigning a query profile - make it overridable return new BackedOverridableQueryProfile((QueryProfile)value); } return value; } @Override - protected QueryProfile createSubProfile(String name,DimensionBinding binding) { + protected QueryProfile createSubProfile(String name, DimensionBinding binding) { return new OverridableQueryProfile(); // Nothing is set in this branch, so nothing to override, but need override checking } @@ -38,7 +38,7 @@ public class OverridableQueryProfile extends QueryProfile { @Override public OverridableQueryProfile clone() { if (isFrozen()) return this; - OverridableQueryProfile clone=(OverridableQueryProfile)super.clone(); + OverridableQueryProfile clone = (OverridableQueryProfile)super.clone(); clone.initId(ComponentId.createAnonymousComponentId(simpleClassName)); return clone; } diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfile.java b/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfile.java index a311b55339c..e9ccdd22f98 100644 --- a/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfile.java +++ b/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfile.java @@ -42,6 +42,9 @@ import java.util.regex.Pattern; */ public class QueryProfile extends FreezableSimpleComponent implements Cloneable { + /** The name of the source of this (a file) */ + private final String source; + /** Defines the permissible content of this, or null if any content is permissible */ private QueryProfileType type = null; @@ -64,7 +67,7 @@ public class QueryProfile extends FreezableSimpleComponent implements Cloneable * Field override settings: fieldName→OverrideValue. These overrides the override * setting in the type (if any) of this field). If there are no query profile level settings, this is null. */ - private Map overridable = null; + private Map overridable = null; /** * Creates a new query profile from an id. @@ -72,9 +75,7 @@ public class QueryProfile extends FreezableSimpleComponent implements Cloneable * At that point it becomes readable but unmodifiable, which it stays until it goes out of reference. */ public QueryProfile(ComponentId id) { - super(id); - if ( ! id.isAnonymous()) - validateName(id.getName()); + this(id, id.stringValue()); } /** Convenience shorthand for new QueryProfile(new ComponentId(idString)) */ @@ -82,10 +83,19 @@ public class QueryProfile extends FreezableSimpleComponent implements Cloneable this(new ComponentId(idString)); } + public QueryProfile(ComponentId id, String sourceName) { + super(id); + this.source = sourceName; + if ( ! id.isAnonymous()) + validateName(id.getName()); + } + // ----------------- Public API ------------------------------------------------------------------------------- // ----------------- Setters and getters + public String getSource() { return source; } + /** Returns the type of this or null if it has no type */ public QueryProfileType getType() { return type; } @@ -723,9 +733,8 @@ public class QueryProfile extends FreezableSimpleComponent implements Cloneable * Looks up all inherited profiles and adds any that matches this name. * This default implementation returns an empty profile. */ - protected QueryProfile createSubProfile(String name,DimensionBinding dimensionBinding) { - QueryProfile queryProfile = new QueryProfile(ComponentId.createAnonymousComponentId(name)); - return queryProfile; + protected QueryProfile createSubProfile(String name, DimensionBinding dimensionBinding) { + return new QueryProfile(ComponentId.createAnonymousComponentId(name), source); } /** Do a variant-aware content lookup in this */ @@ -763,7 +772,7 @@ public class QueryProfile extends FreezableSimpleComponent implements Cloneable ensureNotFrozen(); if (name.isCompound()) { QueryProfile parent = getQueryProfileExact(name.first(), true, dimensionBinding); - parent.setNode(name.rest(), value,parentType, dimensionBinding.createFor(parent.getDimensions()), registry); + parent.setNode(name.rest(), value, parentType, dimensionBinding.createFor(parent.getDimensions()), registry); } else { setLocalNode(name.toString(), value,parentType, dimensionBinding, registry); @@ -796,19 +805,18 @@ public class QueryProfile extends FreezableSimpleComponent implements Cloneable */ private QueryProfile getQueryProfileExact(String localName, boolean create, DimensionBinding dimensionBinding) { Object node = localExactLookup(localName, dimensionBinding); - if (node != null && node instanceof QueryProfile) { - return (QueryProfile)node; - } - if (!create) return null; + if (node instanceof QueryProfile) return (QueryProfile)node; + + if ( ! create) return null; - QueryProfile queryProfile=createSubProfile(localName,dimensionBinding); + QueryProfile queryProfile = createSubProfile(localName,dimensionBinding); if (type != null) { - Class legalClass=type.getValueClass(localName); + Class legalClass = type.getValueClass(localName); if (legalClass == null || ! legalClass.isInstance(queryProfile)) throw new RuntimeException("'" + localName + "' is not a legal query profile reference name in " + this); queryProfile.setType(type.getType(localName)); } - localPut(localName,queryProfile,dimensionBinding); + localPut(localName, queryProfile, dimensionBinding); return queryProfile; } diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/compiled/CompiledQueryProfile.java b/container-search/src/main/java/com/yahoo/search/query/profile/compiled/CompiledQueryProfile.java index 06a6ac48cb6..644d366e7d0 100644 --- a/container-search/src/main/java/com/yahoo/search/query/profile/compiled/CompiledQueryProfile.java +++ b/container-search/src/main/java/com/yahoo/search/query/profile/compiled/CompiledQueryProfile.java @@ -104,6 +104,7 @@ public class CompiledQueryProfile extends AbstractComponent implements Cloneable */ public final Map listValues(CompoundName prefix) { return listValues(prefix, Collections.emptyMap()); } public final Map listValues(String prefix) { return listValues(new CompoundName(prefix)); } + /** * Return all objects that start with the given prefix path. Use "" to list all. *

@@ -113,6 +114,7 @@ public class CompiledQueryProfile extends AbstractComponent implements Cloneable public final Map listValues(String prefix, Map context) { return listValues(new CompoundName(prefix), context); } + /** * Return all objects that start with the given prefix path. Use "" to list all. *

@@ -122,6 +124,7 @@ public class CompiledQueryProfile extends AbstractComponent implements Cloneable public final Map listValues(CompoundName prefix, Map context) { return listValues(prefix, context, null); } + /** * Adds all objects that start with the given path prefix to the given value map. Use "" to list all. *

@@ -147,6 +150,24 @@ public class CompiledQueryProfile extends AbstractComponent implements Cloneable return values; } + public Map listValuesWithSources(CompoundName prefix, + Map context, + Properties substitution) { + Map values = new HashMap<>(); + for (Map.Entry> entry : entries.entrySet()) { + if ( entry.getKey().size() <= prefix.size()) continue; + if ( ! entry.getKey().hasPrefix(prefix)) continue; + + ValueWithSource valueWithSource = entry.getValue().get(context); + if (valueWithSource == null) continue; + + valueWithSource = valueWithSource.withValue(substitute(valueWithSource.value(), context, substitution)); + CompoundName suffixName = entry.getKey().rest(prefix.size()); + values.put(suffixName.toString(), valueWithSource); + } + return values; + } + public final Object get(String name) { return get(name, Collections.emptyMap()); } diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/compiled/ValueWithSource.java b/container-search/src/main/java/com/yahoo/search/query/profile/compiled/ValueWithSource.java index c2ce34c3f47..11281b3cd52 100644 --- a/container-search/src/main/java/com/yahoo/search/query/profile/compiled/ValueWithSource.java +++ b/container-search/src/main/java/com/yahoo/search/query/profile/compiled/ValueWithSource.java @@ -39,7 +39,10 @@ public class ValueWithSource { @Override public String toString() { - return value + " from " + ownerId + ( variant != null ? " variant " + variant : ""); + return value + + " (from query profile '" + ownerId + "'" + + ( variant != null ? " variant " + variant : "") + + ")"; } } diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/config/QueryProfileConfigurer.java b/container-search/src/main/java/com/yahoo/search/query/profile/config/QueryProfileConfigurer.java index 1af78982b9c..d3c232f84c5 100644 --- a/container-search/src/main/java/com/yahoo/search/query/profile/config/QueryProfileConfigurer.java +++ b/container-search/src/main/java/com/yahoo/search/query/profile/config/QueryProfileConfigurer.java @@ -74,7 +74,7 @@ public class QueryProfileConfigurer implements ConfigSubscriber.SingleSubscriber } private static void createProfile(QueryProfilesConfig.Queryprofile config, QueryProfileRegistry registry) { - QueryProfile profile = new QueryProfile(config.id()); + QueryProfile profile = new QueryProfile(new ComponentId(config.id()), config.id()); try { String typeId = config.type(); if (typeId != null && ! typeId.isEmpty()) diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/config/QueryProfileXMLReader.java b/container-search/src/main/java/com/yahoo/search/query/profile/config/QueryProfileXMLReader.java index 210b4899c58..33f07a58195 100644 --- a/container-search/src/main/java/com/yahoo/search/query/profile/config/QueryProfileXMLReader.java +++ b/container-search/src/main/java/com/yahoo/search/query/profile/config/QueryProfileXMLReader.java @@ -135,7 +135,7 @@ public class QueryProfileXMLReader { ComponentId id = new ComponentId(idString); validateFileNameToId(reader.getName(), id, "query profile"); - QueryProfile queryProfile = new QueryProfile(id); + QueryProfile queryProfile = new QueryProfile(id, reader.getName()); String typeId = root.getAttribute("type"); if (typeId != null && ! typeId.equals("")) { QueryProfileType type = registry.getType(typeId); diff --git a/container-search/src/test/java/com/yahoo/search/query/profile/test/QueryProfileTestCase.java b/container-search/src/test/java/com/yahoo/search/query/profile/test/QueryProfileTestCase.java index 3c276157f72..5e2e018f375 100644 --- a/container-search/src/test/java/com/yahoo/search/query/profile/test/QueryProfileTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/query/profile/test/QueryProfileTestCase.java @@ -601,7 +601,7 @@ public class QueryProfileTestCase { Query query = new Query("?foo=value&tracelevel=4", cProfile); assertEquals("value", query.properties().get("property")); - assertTrue(traceContains("foo=value", query)); + assertTrue(traceContains("foo: value", query)); } // NB: NOT RECURSIVE diff --git a/container-search/src/test/java/com/yahoo/search/rendering/JsonRendererTestCase.java b/container-search/src/test/java/com/yahoo/search/rendering/JsonRendererTestCase.java index 1a68f14af06..a83a0e84bbf 100644 --- a/container-search/src/test/java/com/yahoo/search/rendering/JsonRendererTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/rendering/JsonRendererTestCase.java @@ -362,7 +362,7 @@ public class JsonRendererTestCase { + " \"message\": \"No query profile is used\"" + " }," + " {" - + " \"message\": \"Resolved properties:\\ntracelevel=10 (value from request)\\nquery=a (value from request)\\n\"" + + " \"message\": \"Resolved properties:\\ntracelevel: 10 (from request)\\nquery: a (from request)\\n\"" + " }," + " {" + " \"children\": [" @@ -395,7 +395,7 @@ public class JsonRendererTestCase { Map subtrace = (Map) children1.get(2); List children2 = (List) subtrace.get("children"); Map traceElement = (Map) children2.get(0); - traceElement.put("timestamp", Integer.valueOf(42)); + traceElement.put("timestamp", 42); } assertEquals(exp, gen); } diff --git a/container-search/src/test/java/com/yahoo/search/test/QueryTestCase.java b/container-search/src/test/java/com/yahoo/search/test/QueryTestCase.java index fb628156aab..8c6c4e31808 100644 --- a/container-search/src/test/java/com/yahoo/search/test/QueryTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/test/QueryTestCase.java @@ -93,7 +93,6 @@ public class QueryTestCase { and.addItem(new WordItem(token.getTokenString(), "body")); } query.getModel().getQueryTree().setRoot(and); - System.out.println(query); } // TODO: YQL work in progress (jon) @@ -380,8 +379,6 @@ public class QueryTestCase { Query q = new Query(QueryTestCase.httpEncode("?query=a:>5&a=b&traceLevel=5&sources=a,b&u=12&foo.bar2=wiz2&c.d=foo&queryProfile=test"),testProfile.compile(null)); String trace = q.getContext(false).getTrace().toString(); String[] traceLines = trace.split("\n"); - for (String line : traceLines) - System.out.println(line); } @Test @@ -515,12 +512,12 @@ public class QueryTestCase { Query q = new Query(QueryTestCase.httpEncode("?query=a:>5&a=b&traceLevel=5&sources=a,b&u=12&foo.bar2=wiz2&c.d=foo&queryProfile=test"),testProfile.compile(null)); String trace = q.getContext(false).getTrace().toString(); String[] traceLines = trace.split("\n"); - assertTrue(contains("query=a:>5 (value from request)", traceLines)); - assertTrue(contains("traceLevel=5 (value from request)", traceLines)); - assertTrue(contains("a=b (value from request)", traceLines)); - assertTrue(contains("sources=[a, b] (value from request)", traceLines)); - assertTrue(contains("d=e (value from query profile)", traceLines)); - assertTrue(contains("u=11 (value from query profile - unoverridable, ignoring request value)", traceLines)); + assertTrue(contains("query: a:>5 (from request)", traceLines)); + assertTrue(contains("traceLevel: 5 (from request)", traceLines)); + assertTrue(contains("a: b (from request)", traceLines)); + assertTrue(contains("sources: [a, b] (from request)", traceLines)); + assertTrue(contains("d: e (from query profile 'test')", traceLines)); + assertTrue(contains("u: 11 (from query profile - unoverridable, ignoring request value)", traceLines)); } @Test @@ -547,8 +544,8 @@ public class QueryTestCase { Query q = new Query(QueryTestCase.httpEncode("?query=dvd&a.b=foo&tracelevel=9"), defaultProfile.compile(null)); String trace = q.getContext(false).getTrace().toString(); String[] traceLines = trace.split("\n"); - assertTrue(contains("query=dvd (value from request)", traceLines)); - assertTrue(contains("a.b=foo (value from request)", traceLines)); + assertTrue(contains("query: dvd (from request)", traceLines)); + assertTrue(contains("a.b: foo (from request)", traceLines)); } @Test @@ -574,7 +571,7 @@ public class QueryTestCase { assertEquals("a.b-x1-value", propertyList.get("a.b")); String trace = q.getContext(false).getTrace().toString(); String[] traceLines = trace.split("\n"); - assertTrue(contains("a.b=a.b-x1-value (value from query profile)", traceLines)); + assertTrue(contains("a.b: a.b-x1-value (from query profile 'default' variant [x1])", traceLines)); } { @@ -590,7 +587,7 @@ public class QueryTestCase { assertEquals("a.b-x2-value", propertyList.get("a.b")); String trace = q.getContext(false).getTrace().toString(); String[] traceLines = trace.split("\n"); - assertTrue(contains("a.b=a.b-x2-value (value from query profile)", traceLines)); + assertTrue(contains("a.b: a.b-x2-value (from query profile 'default' variant [x2])", traceLines)); } } -- cgit v1.2.3