-
Notifications
You must be signed in to change notification settings - Fork 139
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merges in Armeria example #48
Conversation
.. because they are same for `HttpClient`.
- Armeria 0.64.0 -> 0.68.0 - Brave 4.19.3 -> 5.1.4 - Build-time only - maven-compiler-plugin 3.7.0 -> 3.8.0
Motivation: line/armeria#1322 Modifications: - Update dependencies - Armeria: 0.68.0 -> 0.69.0 - Brave: 5.1.4 -> 5.1.5 - Use `RequestContextCurrentTraceContext.INSTANCE` instead of `MDCCurrentTraceContext.create()` when creating a new `Tracing`
Also: - Remove unused imports - Add `final` where necessary
- Armeria 0.70.0 -> 0.75.0 - Brave 5.1.5 -> 5.5.0
Coming from a question on gitter from @Romender
- Armeria 0.95.0 -> 0.96.0 - Brave 5.8.0 -> 5.9.0
- Add Maven wrapper so that a user can start playing with the example even without local Maven installation. - Update Armeria from 0.96.0 to 0.97.0 - Update Brave from 5.9.0 to 5.9.1
- Armeria 0.97.0 -> 0.98.1 - Brave 5.9.1 -> 5.9.4
- Armeria 0.98.1 -> 0.98.4 - Brave 5.9.4 -> 5.9.5
- Armeria 0.99.8 -> 0.99.9 - Brave 5.12.3 -> 5.12.4
Because `spanReporter(...)` is deprecated.
Armeria 1.0.0 -> 1.1.0 Brave 5.12.4 -> 5.12.6
e2da0cc
to
6a500d9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some notes for the curious:
changes from latest https://github.com/openzipkin-contrib/zipkin-armeria-example
@@ -0,0 +1,36 @@ | |||
# Basic example showing distributed tracing across Armeria apps |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
later we'll revisit the README, but this is the same as it was except no maven wrapper
.service("/health", (ctx, req) -> HttpResponse.of("ok\n")) | ||
.service("/api", (ctx, req) -> { | ||
String response = new Date().toString(); | ||
String username = req.headers().get("user_name"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this part is new, just to match the other examples
|
||
final Server server = Server.builder() | ||
.http(9000) | ||
.service("/health", (ctx, req) -> HttpResponse.of("ok\n")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dummy health endpoint unless armeria has a built-in one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HealthCheckService
might work here. It responds a shiny JSON though.
import zipkin2.reporter.brave.AsyncZipkinSpanHandler; | ||
import zipkin2.reporter.urlconnection.URLConnectionSender; | ||
|
||
final class HttpTracingFactory { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
main differences here are baggage configuration, SLF4J integration and export of HttpTracing
<!-- encoders are assigned the type | ||
ch.qos.logback.classic.encoder.PatternLayoutEncoder by default --> | ||
<encoder> | ||
<pattern>%d{HH:mm:ss.SSS} [%thread] [%X{userName}] [%X{traceId}/%X{spanId}] %-5level %logger{36} - %msg%n</pattern> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pattern now includes userName
|
||
EXPOSE 8081 9000 | ||
|
||
ENV JAVA_OPTS="-Xms16m -Xmx16m -XX:+ExitOnOutOfMemoryError" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm assuming this will work :D
|
||
WORKDIR /brave-example | ||
COPY --from=install /install/webmvc4-jetty/ . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
alphabetical re-ordering
@@ -9,7 +9,7 @@ services: | |||
# Generate traffic by hitting http://localhost:8081 | |||
frontend: | |||
container_name: frontend | |||
image: openzipkin/example-brave:webmvc4-jetty | |||
image: openzipkin/example-brave:armeria |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
switching default :D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉
@@ -11,7 +11,7 @@ jobs: | |||
build-on-linux: | |||
strategy: | |||
matrix: | |||
docker-target: [webmvc25-jetty, webmvc3-jetty, webmvc4-jetty, webmvc4-boot] | |||
docker-target: [armeria, webmvc25-jetty, webmvc3-jetty, webmvc4-boot, webmvc4-jetty] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to be 'A'rmeria
|
||
final Server server = Server.builder() | ||
.http(9000) | ||
.service("/health", (ctx, req) -> HttpResponse.of("ok\n")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HealthCheckService
might work here. It responds a shiny JSON though.
67ca800
to
8635d29
Compare
This merges in code via
I also updated it to have the same conventions as the other examples, including a docker image openzipkin/example-brave:armeria
cc @ikhoon @minwoox @trustin