You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "stillalex (via GitHub)" <gi...@apache.org> on 2023/08/28 19:32:19 UTC
[GitHub] [solr] stillalex opened a new pull request, #1868: SOLR-15367 [9.x] Convert "rid" functionality into a default Tracer
stillalex opened a new pull request, #1868:
URL: https://github.com/apache/solr/pull/1868
https://issues.apache.org/jira/browse/SOLR-15367
<!--
_(If you are a project committer then you may remove some/all of the following template.)_
Before creating a pull request, please file an issue in the ASF Jira system for Solr:
* https://issues.apache.org/jira/projects/SOLR
For something minor (i.e. that wouldn't be worth putting in release notes), you can skip JIRA.
To create a Jira issue, you will need to create an account there first.
The title of the PR should reference the Jira issue number in the form:
* SOLR-####: <short description of problem or changes>
SOLR must be fully capitalized. A short description helps people scanning pull requests for items they can work on.
Properly referencing the issue in the title ensures that Jira is correctly updated with code review comments and commits. -->
# Description
**PR for 9.x branch**
This is a 9.x version of https://github.com/apache/solr/pull/1854 for evaluation.
# Solution
Please provide a short description of the approach taken to implement your solution.
# Tests
Please describe the tests you've developed or run to confirm this patch implements the feature or solves the problem.
# Checklist
Please review the following and check all that apply:
- [ ] I have reviewed the guidelines for [How to Contribute](https://wiki.apache.org/solr/HowToContribute) and my code conforms to the standards described there to the best of my ability.
- [ ] I have created a Jira issue and added the issue ID to my pull request title.
- [ ] I have given Solr maintainers [access](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) to contribute to my PR branch. (optional but recommended)
- [ ] I have developed this patch against the `main` branch.
- [ ] I have run `./gradlew check`.
- [ ] I have added tests for my changes.
- [ ] I have added documentation for the [Reference Guide](https://github.com/apache/solr/tree/main/solr/solr-ref-guide)
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] stillalex commented on a diff in pull request #1868: SOLR-15367 [9.x] Convert "rid" functionality into a default Tracer
Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex commented on code in PR #1868:
URL: https://github.com/apache/solr/pull/1868#discussion_r1312200136
##########
solr/core/src/java/org/apache/solr/util/tracing/SimplePropagator.java:
##########
@@ -139,11 +147,15 @@ public SimplePropagatorSpanBuilder(ScopeManager scopeManager) {
@Override
public Span start() {
if (parent != null) {
- return new SimplePropagatorSpan(parent);
+ if (parent instanceof SimplePropagatorSpan) {
+ return (SimplePropagatorSpan) parent;
+ } else {
+ return NoopSpan.INSTANCE;
+ }
} else if (scopeManager.activeSpan() != null) {
- return new SimplePropagatorSpan(scopeManager.activeSpan().context());
+ return scopeManager.activeSpan();
} else {
Review Comment:
yes. good catch
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] dsmiley commented on a diff in pull request #1868: SOLR-15367 [9.x] Convert "rid" functionality into a default Tracer
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #1868:
URL: https://github.com/apache/solr/pull/1868#discussion_r1310618039
##########
solr/core/src/java/org/apache/solr/util/tracing/SimplePropagator.java:
##########
@@ -0,0 +1,304 @@
+/*
+ * 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
+ *
+ * http://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.solr.util.tracing;
+
+import io.opentracing.Scope;
+import io.opentracing.ScopeManager;
+import io.opentracing.Span;
+import io.opentracing.SpanContext;
+import io.opentracing.Tracer;
+import io.opentracing.Tracer.SpanBuilder;
+import io.opentracing.propagation.Format;
+import io.opentracing.propagation.TextMap;
+import io.opentracing.tag.Tag;
+import io.opentracing.util.GlobalTracer;
+import io.opentracing.util.ThreadLocalScopeManager;
+import java.lang.invoke.MethodHandles;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.concurrent.atomic.AtomicLong;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class SimplePropagator {
+ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ private static final String TRACE_HOST_NAME =
+ System.getProperty("trace_host_name", System.getProperty("host"));
+
+ static final String TRACE_ID = System.getProperty("TRACE_ID", "X-Trace-Id");
+
+ private static final AtomicLong traceCounter = new AtomicLong(0);
+
+ public static void load() {
+ GlobalTracer.registerIfAbsent(
+ () -> {
+ log.info("Always-on trace id generation enabled.");
+ return new SimplePropagatorTracer();
+ });
+ }
+
+ /**
+ * Tracer that only aims to do simple header propagation, tailored to how Solr works.
+ *
+ * <p>Heavily inspired from JaegerTracer, NoopTracer
+ */
+ static class SimplePropagatorTracer implements Tracer {
+
+ private final ScopeManager scopeManager = new ThreadLocalScopeManager();
+
+ @Override
+ public ScopeManager scopeManager() {
+ return scopeManager;
+ }
+
+ @Override
+ public Span activeSpan() {
+ return scopeManager.activeSpan();
+ }
+
+ @Override
+ public Scope activateSpan(Span span) {
+ return scopeManager.activate(span);
+ }
+
+ @Override
+ public SpanBuilder buildSpan(String operationName) {
+ return new SimplePropagatorSpanBuilder(scopeManager);
+ }
+
+ @Override
+ public void close() {}
+
+ @Override
+ public <C> void inject(SpanContext spanContext, Format<C> format, C carrier) {
+ if (!format.equals(Format.Builtin.HTTP_HEADERS)) {
+ throw new IllegalArgumentException("unsupported format " + format);
+ }
+ String traceId = spanContext.toTraceId();
+ if (traceId != null && !traceId.isEmpty()) {
+ TextMap tm = (TextMap) carrier;
+ tm.put(TRACE_ID, traceId);
+ }
+ }
+
+ @Override
+ public <C> SpanContext extract(Format<C> format, C carrier) {
+ if (!format.equals(Format.Builtin.HTTP_HEADERS)) {
+ throw new IllegalArgumentException("unsupported format " + format);
+ }
+ String traceId = null;
+ TextMap tm = (TextMap) carrier;
+ Iterator<Entry<String, String>> it = tm.iterator();
+ while (it.hasNext()) {
+ var e = it.next();
+ // 'equalsIgnoreCase' because header name might be lowercase
+ if (e.getKey().equalsIgnoreCase(TRACE_ID)) {
+ traceId = e.getValue();
+ break;
+ }
+ }
+ if (traceId == null) {
+ traceId = newTraceId();
+ }
+ return new SimplePropagatorSpanContext(traceId);
+ }
+
+ @Override
+ public String toString() {
+ return "SimplePropagator";
+ }
+ }
+
+ private static final class SimplePropagatorSpanBuilder implements SpanBuilder {
+
+ private final ScopeManager scopeManager;
+ // storing parent to support the parent being injected via the asChildOf method
+ private SpanContext parent;
+
+ public SimplePropagatorSpanBuilder(ScopeManager scopeManager) {
+ this.scopeManager = scopeManager;
+ }
+
+ @Override
+ public Span start() {
+ if (parent != null) {
+ return new SimplePropagatorSpan(parent);
+ } else if (scopeManager.activeSpan() != null) {
+ return new SimplePropagatorSpan(scopeManager.activeSpan().context());
+ } else {
+ return new SimplePropagatorSpan(new SimplePropagatorSpanContext(""));
+ }
+ }
+
+ @Override
+ public SpanBuilder addReference(String referenceType, SpanContext reference) {
+ return this;
+ }
+
+ @Override
+ public SpanBuilder asChildOf(SpanContext parent) {
+ this.parent = parent;
+ return this;
+ }
+
+ @Override
+ public SpanBuilder asChildOf(Span parent) {
+ return this;
+ }
+
+ @Override
+ public SpanBuilder ignoreActiveSpan() {
+ return this;
+ }
+
+ @Override
+ public SpanBuilder withStartTimestamp(long arg0) {
+ return this;
+ }
+
+ @Override
+ public SpanBuilder withTag(String arg0, String arg1) {
+ return this;
+ }
+
+ @Override
+ public SpanBuilder withTag(String arg0, boolean arg1) {
+ return this;
+ }
+
+ @Override
+ public SpanBuilder withTag(String arg0, Number arg1) {
+ return this;
+ }
+
+ @Override
+ public <T> SpanBuilder withTag(Tag<T> arg0, T arg1) {
+ return this;
+ }
+ }
+
+ private static final class SimplePropagatorSpanContext implements SpanContext {
+
+ private final String traceId;
+
+ public SimplePropagatorSpanContext(String traceId) {
+ this.traceId = traceId;
+ }
+
+ @Override
+ public String toTraceId() {
+ return traceId;
+ }
+
+ @Override
+ public String toSpanId() {
+ return "";
+ }
+
+ @Override
+ public Iterable<Map.Entry<String, String>> baggageItems() {
+ return Collections.emptyList();
+ }
+
+ @Override
+ public String toString() {
+ return "SimplePropagatorSpanContext [traceId=" + traceId + "]";
+ }
+ }
+
+ static final class SimplePropagatorSpan implements Span {
+
+ private final SpanContext ctx;
+
+ private SimplePropagatorSpan(SpanContext ctx) {
+ this.ctx = ctx;
+ }
+
+ @Override
+ public SpanContext context() {
+ return ctx;
+ }
+
+ @Override
+ public void finish() {}
+
+ @Override
+ public void finish(long arg0) {}
+
+ @Override
+ public String getBaggageItem(String arg0) {
+ return null;
+ }
+
+ @Override
+ public Span log(Map<String, ?> arg0) {
+ return this;
+ }
+
+ @Override
+ public Span log(String arg0) {
+ return this;
+ }
+
+ @Override
+ public Span log(long arg0, Map<String, ?> arg1) {
+ return this;
+ }
+
+ @Override
+ public Span log(long arg0, String arg1) {
+ return this;
+ }
+
+ @Override
+ public Span setBaggageItem(String arg0, String arg1) {
+ return this;
+ }
+
+ @Override
+ public Span setOperationName(String arg0) {
+ return this;
+ }
+
+ @Override
+ public Span setTag(String arg0, String arg1) {
+ return this;
+ }
+
+ @Override
+ public Span setTag(String arg0, boolean arg1) {
+ return this;
+ }
+
+ @Override
+ public Span setTag(String arg0, Number arg1) {
+ return this;
+ }
+
+ @Override
+ public <T> Span setTag(Tag<T> arg0, T arg1) {
+ return this;
+ }
+ }
+
+ private static String newTraceId() {
Review Comment:
nit: this method seems to hide down here after these long inner subclasses. Above them is probably better organized.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] dsmiley commented on a diff in pull request #1868: SOLR-15367 [9.x] Convert "rid" functionality into a default Tracer
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #1868:
URL: https://github.com/apache/solr/pull/1868#discussion_r1310608641
##########
solr/core/src/java/org/apache/solr/util/tracing/SimplePropagator.java:
##########
@@ -0,0 +1,304 @@
+/*
+ * 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
+ *
+ * http://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.solr.util.tracing;
+
+import io.opentracing.Scope;
+import io.opentracing.ScopeManager;
+import io.opentracing.Span;
+import io.opentracing.SpanContext;
+import io.opentracing.Tracer;
+import io.opentracing.Tracer.SpanBuilder;
+import io.opentracing.propagation.Format;
+import io.opentracing.propagation.TextMap;
+import io.opentracing.tag.Tag;
+import io.opentracing.util.GlobalTracer;
+import io.opentracing.util.ThreadLocalScopeManager;
+import java.lang.invoke.MethodHandles;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.concurrent.atomic.AtomicLong;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class SimplePropagator {
+ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ private static final String TRACE_HOST_NAME =
+ System.getProperty("trace_host_name", System.getProperty("host"));
+
+ static final String TRACE_ID = System.getProperty("TRACE_ID", "X-Trace-Id");
+
+ private static final AtomicLong traceCounter = new AtomicLong(0);
+
+ public static void load() {
+ GlobalTracer.registerIfAbsent(
+ () -> {
+ log.info("Always-on trace id generation enabled.");
+ return new SimplePropagatorTracer();
+ });
+ }
+
+ /**
+ * Tracer that only aims to do simple header propagation, tailored to how Solr works.
+ *
+ * <p>Heavily inspired from JaegerTracer, NoopTracer
+ */
+ static class SimplePropagatorTracer implements Tracer {
+
+ private final ScopeManager scopeManager = new ThreadLocalScopeManager();
+
+ @Override
+ public ScopeManager scopeManager() {
+ return scopeManager;
+ }
+
+ @Override
+ public Span activeSpan() {
+ return scopeManager.activeSpan();
+ }
+
+ @Override
+ public Scope activateSpan(Span span) {
+ return scopeManager.activate(span);
+ }
+
+ @Override
+ public SpanBuilder buildSpan(String operationName) {
+ return new SimplePropagatorSpanBuilder(scopeManager);
+ }
+
+ @Override
+ public void close() {}
+
+ @Override
+ public <C> void inject(SpanContext spanContext, Format<C> format, C carrier) {
+ if (!format.equals(Format.Builtin.HTTP_HEADERS)) {
+ throw new IllegalArgumentException("unsupported format " + format);
+ }
+ String traceId = spanContext.toTraceId();
+ if (traceId != null && !traceId.isEmpty()) {
+ TextMap tm = (TextMap) carrier;
+ tm.put(TRACE_ID, traceId);
+ }
+ }
+
+ @Override
+ public <C> SpanContext extract(Format<C> format, C carrier) {
+ if (!format.equals(Format.Builtin.HTTP_HEADERS)) {
+ throw new IllegalArgumentException("unsupported format " + format);
+ }
+ String traceId = null;
+ TextMap tm = (TextMap) carrier;
+ Iterator<Entry<String, String>> it = tm.iterator();
+ while (it.hasNext()) {
+ var e = it.next();
+ // 'equalsIgnoreCase' because header name might be lowercase
+ if (e.getKey().equalsIgnoreCase(TRACE_ID)) {
+ traceId = e.getValue();
+ break;
+ }
+ }
+ if (traceId == null) {
+ traceId = newTraceId();
+ }
+ return new SimplePropagatorSpanContext(traceId);
+ }
+
+ @Override
+ public String toString() {
+ return "SimplePropagator";
+ }
+ }
+
+ private static final class SimplePropagatorSpanBuilder implements SpanBuilder {
+
+ private final ScopeManager scopeManager;
+ // storing parent to support the parent being injected via the asChildOf method
+ private SpanContext parent;
+
+ public SimplePropagatorSpanBuilder(ScopeManager scopeManager) {
+ this.scopeManager = scopeManager;
+ }
+
+ @Override
+ public Span start() {
+ if (parent != null) {
+ return new SimplePropagatorSpan(parent);
+ } else if (scopeManager.activeSpan() != null) {
+ return new SimplePropagatorSpan(scopeManager.activeSpan().context());
Review Comment:
This is going to be the common case as more spans show up _within_ Solr. Could SimplePropagatorSpan and SimplePropagatorSpanContext be combined into one? Create no new object; the context could *be* the span if all we are doing is propagation.
I even wonder if we need a distinct builder?
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] stillalex commented on pull request #1868: SOLR-15367 [9.x] Convert "rid" functionality into a default Tracer
Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex commented on PR #1868:
URL: https://github.com/apache/solr/pull/1868#issuecomment-1696261185
@dsmiley raising this PR for a discussion on building an OpenTracing version of the always-on tracer that went into main branch based on OTEL.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] stillalex commented on pull request #1868: SOLR-15367 [9.x] Convert "rid" functionality into a default Tracer
Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex commented on PR #1868:
URL: https://github.com/apache/solr/pull/1868#issuecomment-1709448692
@dsmiley would like to merge this soon. do you think the context injection/extraction parts are correct now?
and apologies for the force merge, I wanted to rebase on top of other backports and ended up with this.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] stillalex commented on a diff in pull request #1868: SOLR-15367 [9.x] Convert "rid" functionality into a default Tracer
Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex commented on code in PR #1868:
URL: https://github.com/apache/solr/pull/1868#discussion_r1311921932
##########
solr/core/src/java/org/apache/solr/util/tracing/SimplePropagator.java:
##########
@@ -0,0 +1,304 @@
+/*
+ * 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
+ *
+ * http://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.solr.util.tracing;
+
+import io.opentracing.Scope;
+import io.opentracing.ScopeManager;
+import io.opentracing.Span;
+import io.opentracing.SpanContext;
+import io.opentracing.Tracer;
+import io.opentracing.Tracer.SpanBuilder;
+import io.opentracing.propagation.Format;
+import io.opentracing.propagation.TextMap;
+import io.opentracing.tag.Tag;
+import io.opentracing.util.GlobalTracer;
+import io.opentracing.util.ThreadLocalScopeManager;
+import java.lang.invoke.MethodHandles;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.concurrent.atomic.AtomicLong;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class SimplePropagator {
+ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ private static final String TRACE_HOST_NAME =
+ System.getProperty("trace_host_name", System.getProperty("host"));
+
+ static final String TRACE_ID = System.getProperty("TRACE_ID", "X-Trace-Id");
+
+ private static final AtomicLong traceCounter = new AtomicLong(0);
+
+ public static void load() {
+ GlobalTracer.registerIfAbsent(
+ () -> {
+ log.info("Always-on trace id generation enabled.");
+ return new SimplePropagatorTracer();
+ });
+ }
+
+ /**
+ * Tracer that only aims to do simple header propagation, tailored to how Solr works.
+ *
+ * <p>Heavily inspired from JaegerTracer, NoopTracer
+ */
+ static class SimplePropagatorTracer implements Tracer {
+
+ private final ScopeManager scopeManager = new ThreadLocalScopeManager();
+
+ @Override
+ public ScopeManager scopeManager() {
+ return scopeManager;
+ }
+
+ @Override
+ public Span activeSpan() {
+ return scopeManager.activeSpan();
+ }
+
+ @Override
+ public Scope activateSpan(Span span) {
+ return scopeManager.activate(span);
+ }
+
+ @Override
+ public SpanBuilder buildSpan(String operationName) {
+ return new SimplePropagatorSpanBuilder(scopeManager);
+ }
+
+ @Override
+ public void close() {}
+
+ @Override
+ public <C> void inject(SpanContext spanContext, Format<C> format, C carrier) {
+ if (!format.equals(Format.Builtin.HTTP_HEADERS)) {
+ throw new IllegalArgumentException("unsupported format " + format);
+ }
Review Comment:
honestly not sure how to approach this so it works on a generic case. I will return change to early in case this is not a known format (the one Solr sets up) instead of throwing.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] dsmiley commented on a diff in pull request #1868: SOLR-15367 [9.x] Convert "rid" functionality into a default Tracer
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #1868:
URL: https://github.com/apache/solr/pull/1868#discussion_r1312162134
##########
solr/core/src/java/org/apache/solr/util/tracing/SimplePropagator.java:
##########
@@ -139,11 +147,15 @@ public SimplePropagatorSpanBuilder(ScopeManager scopeManager) {
@Override
public Span start() {
if (parent != null) {
- return new SimplePropagatorSpan(parent);
+ if (parent instanceof SimplePropagatorSpan) {
+ return (SimplePropagatorSpan) parent;
+ } else {
+ return NoopSpan.INSTANCE;
+ }
} else if (scopeManager.activeSpan() != null) {
- return new SimplePropagatorSpan(scopeManager.activeSpan().context());
+ return scopeManager.activeSpan();
} else {
Review Comment:
I think this is a double-lookup in a ThreadLocal?
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] dsmiley commented on a diff in pull request #1868: SOLR-15367 [9.x] Convert "rid" functionality into a default Tracer
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #1868:
URL: https://github.com/apache/solr/pull/1868#discussion_r1310615888
##########
solr/core/src/java/org/apache/solr/util/tracing/SimplePropagator.java:
##########
@@ -0,0 +1,304 @@
+/*
+ * 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
+ *
+ * http://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.solr.util.tracing;
+
+import io.opentracing.Scope;
+import io.opentracing.ScopeManager;
+import io.opentracing.Span;
+import io.opentracing.SpanContext;
+import io.opentracing.Tracer;
+import io.opentracing.Tracer.SpanBuilder;
+import io.opentracing.propagation.Format;
+import io.opentracing.propagation.TextMap;
+import io.opentracing.tag.Tag;
+import io.opentracing.util.GlobalTracer;
+import io.opentracing.util.ThreadLocalScopeManager;
+import java.lang.invoke.MethodHandles;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.concurrent.atomic.AtomicLong;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class SimplePropagator {
+ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ private static final String TRACE_HOST_NAME =
+ System.getProperty("trace_host_name", System.getProperty("host"));
+
+ static final String TRACE_ID = System.getProperty("TRACE_ID", "X-Trace-Id");
+
+ private static final AtomicLong traceCounter = new AtomicLong(0);
+
+ public static void load() {
+ GlobalTracer.registerIfAbsent(
+ () -> {
+ log.info("Always-on trace id generation enabled.");
+ return new SimplePropagatorTracer();
+ });
+ }
+
+ /**
+ * Tracer that only aims to do simple header propagation, tailored to how Solr works.
+ *
+ * <p>Heavily inspired from JaegerTracer, NoopTracer
+ */
+ static class SimplePropagatorTracer implements Tracer {
+
+ private final ScopeManager scopeManager = new ThreadLocalScopeManager();
+
+ @Override
+ public ScopeManager scopeManager() {
+ return scopeManager;
+ }
+
+ @Override
+ public Span activeSpan() {
+ return scopeManager.activeSpan();
+ }
+
+ @Override
+ public Scope activateSpan(Span span) {
+ return scopeManager.activate(span);
+ }
+
+ @Override
+ public SpanBuilder buildSpan(String operationName) {
+ return new SimplePropagatorSpanBuilder(scopeManager);
+ }
+
+ @Override
+ public void close() {}
+
+ @Override
+ public <C> void inject(SpanContext spanContext, Format<C> format, C carrier) {
+ if (!format.equals(Format.Builtin.HTTP_HEADERS)) {
+ throw new IllegalArgumentException("unsupported format " + format);
+ }
Review Comment:
Is this constraint necessary? What if we inject & extract to transfer to the Overseer? (I have done this at work)
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] stillalex commented on a diff in pull request #1868: SOLR-15367 [9.x] Convert "rid" functionality into a default Tracer
Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex commented on code in PR #1868:
URL: https://github.com/apache/solr/pull/1868#discussion_r1313379893
##########
solr/core/src/java/org/apache/solr/util/tracing/SimplePropagator.java:
##########
@@ -0,0 +1,304 @@
+/*
+ * 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
+ *
+ * http://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.solr.util.tracing;
+
+import io.opentracing.Scope;
+import io.opentracing.ScopeManager;
+import io.opentracing.Span;
+import io.opentracing.SpanContext;
+import io.opentracing.Tracer;
+import io.opentracing.Tracer.SpanBuilder;
+import io.opentracing.propagation.Format;
+import io.opentracing.propagation.TextMap;
+import io.opentracing.tag.Tag;
+import io.opentracing.util.GlobalTracer;
+import io.opentracing.util.ThreadLocalScopeManager;
+import java.lang.invoke.MethodHandles;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.concurrent.atomic.AtomicLong;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class SimplePropagator {
+ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ private static final String TRACE_HOST_NAME =
+ System.getProperty("trace_host_name", System.getProperty("host"));
+
+ static final String TRACE_ID = System.getProperty("TRACE_ID", "X-Trace-Id");
+
+ private static final AtomicLong traceCounter = new AtomicLong(0);
+
+ public static void load() {
+ GlobalTracer.registerIfAbsent(
+ () -> {
+ log.info("Always-on trace id generation enabled.");
+ return new SimplePropagatorTracer();
+ });
+ }
+
+ /**
+ * Tracer that only aims to do simple header propagation, tailored to how Solr works.
+ *
+ * <p>Heavily inspired from JaegerTracer, NoopTracer
+ */
+ static class SimplePropagatorTracer implements Tracer {
+
+ private final ScopeManager scopeManager = new ThreadLocalScopeManager();
+
+ @Override
+ public ScopeManager scopeManager() {
+ return scopeManager;
+ }
+
+ @Override
+ public Span activeSpan() {
+ return scopeManager.activeSpan();
+ }
+
+ @Override
+ public Scope activateSpan(Span span) {
+ return scopeManager.activate(span);
+ }
+
+ @Override
+ public SpanBuilder buildSpan(String operationName) {
+ return new SimplePropagatorSpanBuilder(scopeManager);
+ }
+
+ @Override
+ public void close() {}
+
+ @Override
+ public <C> void inject(SpanContext spanContext, Format<C> format, C carrier) {
+ if (!format.equals(Format.Builtin.HTTP_HEADERS)) {
+ throw new IllegalArgumentException("unsupported format " + format);
+ }
Review Comment:
> I don't think need to care about what the format is; just try to inject best-effort.
I agree. the `inject` is easy enough, you just return early. the `extract` is now a `return NoopSpan.INSTANCE.context();` and this I'm not sure will work correctly.
not having some basic test coverage for this setup worries me a bit.
##########
solr/core/src/java/org/apache/solr/util/tracing/SimplePropagator.java:
##########
@@ -0,0 +1,304 @@
+/*
+ * 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
+ *
+ * http://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.solr.util.tracing;
+
+import io.opentracing.Scope;
+import io.opentracing.ScopeManager;
+import io.opentracing.Span;
+import io.opentracing.SpanContext;
+import io.opentracing.Tracer;
+import io.opentracing.Tracer.SpanBuilder;
+import io.opentracing.propagation.Format;
+import io.opentracing.propagation.TextMap;
+import io.opentracing.tag.Tag;
+import io.opentracing.util.GlobalTracer;
+import io.opentracing.util.ThreadLocalScopeManager;
+import java.lang.invoke.MethodHandles;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.concurrent.atomic.AtomicLong;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class SimplePropagator {
+ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ private static final String TRACE_HOST_NAME =
+ System.getProperty("trace_host_name", System.getProperty("host"));
+
+ static final String TRACE_ID = System.getProperty("TRACE_ID", "X-Trace-Id");
+
+ private static final AtomicLong traceCounter = new AtomicLong(0);
+
+ public static void load() {
+ GlobalTracer.registerIfAbsent(
+ () -> {
+ log.info("Always-on trace id generation enabled.");
+ return new SimplePropagatorTracer();
+ });
+ }
+
+ /**
+ * Tracer that only aims to do simple header propagation, tailored to how Solr works.
+ *
+ * <p>Heavily inspired from JaegerTracer, NoopTracer
+ */
+ static class SimplePropagatorTracer implements Tracer {
+
+ private final ScopeManager scopeManager = new ThreadLocalScopeManager();
+
+ @Override
+ public ScopeManager scopeManager() {
+ return scopeManager;
+ }
+
+ @Override
+ public Span activeSpan() {
+ return scopeManager.activeSpan();
+ }
+
+ @Override
+ public Scope activateSpan(Span span) {
+ return scopeManager.activate(span);
+ }
+
+ @Override
+ public SpanBuilder buildSpan(String operationName) {
+ return new SimplePropagatorSpanBuilder(scopeManager);
+ }
+
+ @Override
+ public void close() {}
+
+ @Override
+ public <C> void inject(SpanContext spanContext, Format<C> format, C carrier) {
+ if (!format.equals(Format.Builtin.HTTP_HEADERS)) {
+ throw new IllegalArgumentException("unsupported format " + format);
+ }
Review Comment:
> I don't think need to care about what the format is; just try to inject best-effort.
I agree. the `inject` is easy enough, you just return early. the `extract` is now a `return NoopSpan.INSTANCE.context();` and this I'm not sure will work correctly.
not having some basic test coverage for this setup worries me a bit.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] stillalex commented on a diff in pull request #1868: SOLR-15367 [9.x] Convert "rid" functionality into a default Tracer
Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex commented on code in PR #1868:
URL: https://github.com/apache/solr/pull/1868#discussion_r1311901790
##########
solr/core/src/java/org/apache/solr/util/tracing/TraceUtils.java:
##########
@@ -32,7 +33,7 @@ public static void setDbInstance(SolrQueryRequest req, String coreOrColl) {
}
public static void ifNotNoop(Span span, Consumer<Span> consumer) {
- if (span != null && !(span instanceof NoopSpan)) {
+ if (span != null && !(span instanceof NoopSpan) && !(span instanceof SimplePropagatorSpan)) {
Review Comment:
ugh, of course, good one
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] stillalex merged pull request #1868: SOLR-15367 [9.x] Convert "rid" functionality into a default Tracer
Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex merged PR #1868:
URL: https://github.com/apache/solr/pull/1868
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] dsmiley commented on a diff in pull request #1868: SOLR-15367 [9.x] Convert "rid" functionality into a default Tracer
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #1868:
URL: https://github.com/apache/solr/pull/1868#discussion_r1313353614
##########
solr/core/src/java/org/apache/solr/util/tracing/SimplePropagator.java:
##########
@@ -0,0 +1,304 @@
+/*
+ * 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
+ *
+ * http://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.solr.util.tracing;
+
+import io.opentracing.Scope;
+import io.opentracing.ScopeManager;
+import io.opentracing.Span;
+import io.opentracing.SpanContext;
+import io.opentracing.Tracer;
+import io.opentracing.Tracer.SpanBuilder;
+import io.opentracing.propagation.Format;
+import io.opentracing.propagation.TextMap;
+import io.opentracing.tag.Tag;
+import io.opentracing.util.GlobalTracer;
+import io.opentracing.util.ThreadLocalScopeManager;
+import java.lang.invoke.MethodHandles;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.concurrent.atomic.AtomicLong;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class SimplePropagator {
+ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ private static final String TRACE_HOST_NAME =
+ System.getProperty("trace_host_name", System.getProperty("host"));
+
+ static final String TRACE_ID = System.getProperty("TRACE_ID", "X-Trace-Id");
+
+ private static final AtomicLong traceCounter = new AtomicLong(0);
+
+ public static void load() {
+ GlobalTracer.registerIfAbsent(
+ () -> {
+ log.info("Always-on trace id generation enabled.");
+ return new SimplePropagatorTracer();
+ });
+ }
+
+ /**
+ * Tracer that only aims to do simple header propagation, tailored to how Solr works.
+ *
+ * <p>Heavily inspired from JaegerTracer, NoopTracer
+ */
+ static class SimplePropagatorTracer implements Tracer {
+
+ private final ScopeManager scopeManager = new ThreadLocalScopeManager();
+
+ @Override
+ public ScopeManager scopeManager() {
+ return scopeManager;
+ }
+
+ @Override
+ public Span activeSpan() {
+ return scopeManager.activeSpan();
+ }
+
+ @Override
+ public Scope activateSpan(Span span) {
+ return scopeManager.activate(span);
+ }
+
+ @Override
+ public SpanBuilder buildSpan(String operationName) {
+ return new SimplePropagatorSpanBuilder(scopeManager);
+ }
+
+ @Override
+ public void close() {}
+
+ @Override
+ public <C> void inject(SpanContext spanContext, Format<C> format, C carrier) {
+ if (!format.equals(Format.Builtin.HTTP_HEADERS)) {
+ throw new IllegalArgumentException("unsupported format " + format);
+ }
Review Comment:
In practice at the moment, the format will *always* be HTTP since we control where we inject. But if we get to add the Overseer as you have done in main branch, then it will mysteriously not propagate, and then maybe we'll "fix" it by removing these 3 lines. I don't think need to care about what the format is; just try to inject best-effort.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] stillalex commented on pull request #1868: SOLR-15367 [9.x] Convert "rid" functionality into a default Tracer
Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex commented on PR #1868:
URL: https://github.com/apache/solr/pull/1868#issuecomment-1710101561
thank you @dsmiley I really appreciate your diligence here. very happy to see this land on 9.x too!
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] dsmiley commented on a diff in pull request #1868: SOLR-15367 [9.x] Convert "rid" functionality into a default Tracer
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #1868:
URL: https://github.com/apache/solr/pull/1868#discussion_r1318070066
##########
solr/test-framework/src/java/org/apache/solr/cloud/MiniSolrCloudCluster.java:
##########
@@ -1341,5 +1349,10 @@ public Builder withDefaultClusterProperty(String key, String value) {
cluster.put(key, value);
return this;
}
+
Review Comment:
Can you please add javadoc here; it may not be obvious what it does and why a test might want to set it.
I'm thinking maybe:
> Disables the default/built-in simple trace ID generation/propagation. For example if you want to use a MockTracer or a "real" Tracer, you'll need to call this so that it doesn't displace the Tracer you want to use.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] stillalex commented on a diff in pull request #1868: SOLR-15367 [9.x] Convert "rid" functionality into a default Tracer
Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex commented on code in PR #1868:
URL: https://github.com/apache/solr/pull/1868#discussion_r1312007618
##########
solr/core/src/java/org/apache/solr/util/tracing/SimplePropagator.java:
##########
@@ -0,0 +1,304 @@
+/*
+ * 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
+ *
+ * http://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.solr.util.tracing;
+
+import io.opentracing.Scope;
+import io.opentracing.ScopeManager;
+import io.opentracing.Span;
+import io.opentracing.SpanContext;
+import io.opentracing.Tracer;
+import io.opentracing.Tracer.SpanBuilder;
+import io.opentracing.propagation.Format;
+import io.opentracing.propagation.TextMap;
+import io.opentracing.tag.Tag;
+import io.opentracing.util.GlobalTracer;
+import io.opentracing.util.ThreadLocalScopeManager;
+import java.lang.invoke.MethodHandles;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.concurrent.atomic.AtomicLong;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class SimplePropagator {
+ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ private static final String TRACE_HOST_NAME =
+ System.getProperty("trace_host_name", System.getProperty("host"));
+
+ static final String TRACE_ID = System.getProperty("TRACE_ID", "X-Trace-Id");
+
+ private static final AtomicLong traceCounter = new AtomicLong(0);
+
+ public static void load() {
+ GlobalTracer.registerIfAbsent(
+ () -> {
+ log.info("Always-on trace id generation enabled.");
+ return new SimplePropagatorTracer();
+ });
+ }
+
+ /**
+ * Tracer that only aims to do simple header propagation, tailored to how Solr works.
+ *
+ * <p>Heavily inspired from JaegerTracer, NoopTracer
+ */
+ static class SimplePropagatorTracer implements Tracer {
+
+ private final ScopeManager scopeManager = new ThreadLocalScopeManager();
+
+ @Override
+ public ScopeManager scopeManager() {
+ return scopeManager;
+ }
+
+ @Override
+ public Span activeSpan() {
+ return scopeManager.activeSpan();
+ }
+
+ @Override
+ public Scope activateSpan(Span span) {
+ return scopeManager.activate(span);
+ }
+
+ @Override
+ public SpanBuilder buildSpan(String operationName) {
+ return new SimplePropagatorSpanBuilder(scopeManager);
+ }
+
+ @Override
+ public void close() {}
+
+ @Override
+ public <C> void inject(SpanContext spanContext, Format<C> format, C carrier) {
+ if (!format.equals(Format.Builtin.HTTP_HEADERS)) {
+ throw new IllegalArgumentException("unsupported format " + format);
+ }
+ String traceId = spanContext.toTraceId();
+ if (traceId != null && !traceId.isEmpty()) {
+ TextMap tm = (TextMap) carrier;
+ tm.put(TRACE_ID, traceId);
+ }
+ }
+
+ @Override
+ public <C> SpanContext extract(Format<C> format, C carrier) {
+ if (!format.equals(Format.Builtin.HTTP_HEADERS)) {
+ throw new IllegalArgumentException("unsupported format " + format);
+ }
+ String traceId = null;
+ TextMap tm = (TextMap) carrier;
+ Iterator<Entry<String, String>> it = tm.iterator();
+ while (it.hasNext()) {
+ var e = it.next();
+ // 'equalsIgnoreCase' because header name might be lowercase
+ if (e.getKey().equalsIgnoreCase(TRACE_ID)) {
+ traceId = e.getValue();
+ break;
+ }
+ }
+ if (traceId == null) {
+ traceId = newTraceId();
+ }
+ return new SimplePropagatorSpanContext(traceId);
+ }
+
+ @Override
+ public String toString() {
+ return "SimplePropagator";
+ }
+ }
+
+ private static final class SimplePropagatorSpanBuilder implements SpanBuilder {
+
+ private final ScopeManager scopeManager;
+ // storing parent to support the parent being injected via the asChildOf method
+ private SpanContext parent;
+
+ public SimplePropagatorSpanBuilder(ScopeManager scopeManager) {
+ this.scopeManager = scopeManager;
+ }
+
+ @Override
+ public Span start() {
+ if (parent != null) {
+ return new SimplePropagatorSpan(parent);
+ } else if (scopeManager.activeSpan() != null) {
+ return new SimplePropagatorSpan(scopeManager.activeSpan().context());
Review Comment:
good suggestion. I merged the 2 but it introduced some minor complexity dealing with the parent context, but not a big issue.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] dsmiley commented on a diff in pull request #1868: SOLR-15367 [9.x] Convert "rid" functionality into a default Tracer
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #1868:
URL: https://github.com/apache/solr/pull/1868#discussion_r1310619260
##########
solr/core/src/java/org/apache/solr/util/tracing/TraceUtils.java:
##########
@@ -32,7 +33,7 @@ public static void setDbInstance(SolrQueryRequest req, String coreOrColl) {
}
public static void ifNotNoop(Span span, Consumer<Span> consumer) {
- if (span != null && !(span instanceof NoopSpan)) {
+ if (span != null && !(span instanceof NoopSpan) && !(span instanceof SimplePropagatorSpan)) {
Review Comment:
Arguably, SimplePropagatorSpan should implement NoopSpan?
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org