ctubbsii commented on code in PR #453:
URL: https://github.com/apache/accumulo-website/pull/453#discussion_r2036177173


##########
_posts/blog/2022-06-22-2.1.0-metrics-and-tracing.md:
##########
@@ -149,3 +151,27 @@ docker run -d --rm --name jaeger \
 8. View traces in Jaeger UI at http://localhost:16686. You can select the 
service name on the left panel and click `Find Traces` to view the trace 
information. If everything is working correctly, then you should see something 
like the image below.
 
 ![Jaeger 
Screenshot](/images/blog/202206_metrics_and_tracing/Jaeger_Screenshot.png)
+
+## Note for Recent Accumulo Versions (April 2025)
+
+**Note:** This section replaces steps 5 and 6 in the [Tracing 
Example](#tracing-example) above. Follow steps 1-4 from the original 
instructions, then use the updated configuration below instead of steps 5-6, 
and continue with steps 7-8 to view traces in Jaeger.

Review Comment:
   This all seems fine, but is this really a note for recent Accumulo versions, 
or is it a note for newer OpenTelemetry versions? Somebody could use the newer 
OpenTelemetry libraries with the older Accumulo releases, in which case, these 
instructions would apply, right? Likewise, somebody could use a newer version 
of Accumulo but with an older version of OpenTelemetry, in which case the older 
instructions would apply?
   
   I think the instructions are clear, but it is not clear to me that the 
conditions under which you use them are correctly or clearly being 
communicated. I think the version of Open Telemetry is much more important than 
the version of Accumulo for this. And, I think it's wrong to assume that the 
versions in Accumulo's POMs are the authoritative source for what the user is 
(or should be) running.
   
   Users are responsible for their own CLASSPATH, including updating them, even 
for older versions of Accumulo, in response to CVEs, bug fixes, dependency 
convergence, integration issues, etc. That is a downstream responsibility. So, 
we should be careful to word things without making assumptions that the 
dependencies we reference in our POM are what users are, or should be, using.
   
   It might be more correct and clear to say something like: "Accumulo X was 
released compiled and tested with OpenTelemetry Y, which uses the Z property to 
configure...", rather than say "For Accumulo X, you need to set Z property 
...."; the latter might be true, but only for very specific assumptions.



-- 
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: notifications-unsubscr...@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to