Add extension functions and example for OpenTelemetry #187
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
detekt found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
|
@garthgilmourni, please also take a look |
There was a problem hiding this comment.
Hey @marychatte, good job on this!
LGTM in general, please address some small comments before merging
opentelemetry/client/src/main/kotlin/opentelemetry/ktor/example/Client.kt
Outdated
Show resolved
Hide resolved
opentelemetry/client/src/main/kotlin/opentelemetry/ktor/example/Client.kt
Outdated
Show resolved
Hide resolved
opentelemetry/client/src/main/kotlin/opentelemetry/ktor/example/Requests.kt
Show resolved
Hide resolved
...main/kotlin/opentelemetry/ktor/example/plugins/opentelemetry/configureOpenTelemetryServer.kt
Outdated
Show resolved
Hide resolved
| fun Application.configureRouting() { | ||
| install(WebSockets) | ||
|
|
||
| val openTelemetry = installOpenTelemetryOnServer() |
There was a problem hiding this comment.
Can it be an extension on application (stored in the attributes)?
There was a problem hiding this comment.
Thanks for the review! I'm not sure about an extension on the application, because users should provide an instance of OpenTelemetry by themselves (in our example we do it in fun getOpenTelemetry(...) from utils.kt). But please correct me if I misunderstood you
|
@vnikolova, could you please check readme files? |
There was a problem hiding this comment.
That's a pretty cool example! Regarding the README, I think it makes sense to do the following reordering of the sections:
-
This paragraph goes in first: "OpenTelemetry provides support for Ktor with the
KtorClientTracingandKtorServerTracingplugins for the Ktor client and server respectively. For the source code, see the repository on GitHub. " -
then goes the content from the "Motivation" section, without the title.
-
"Running"
-
the content from "Examples", without the title.
Please see my comments for a few more small edits to improve readability :)
opentelemetry/README.md
Outdated
|
|
||
| **Note:** You need to have [Docker](https://www.docker.com/) installed and running to run the sample. | ||
|
|
||
| To run a sample, first, execute the following command in an `opentelemetry` directory: |
There was a problem hiding this comment.
Edit: "To run this sample, execute the following command from the opentelemetry directory:"
opentelemetry/README.md
Outdated
| ./gradlew :client:run | ||
| ``` | ||
|
|
||
| [OpenTelemetry](https://opentelemetry.io/) has support for `Ktor`, you can find source |
There was a problem hiding this comment.
This feels like an introduction. Can we move it to the top of the document?
opentelemetry/README.md
Outdated
| ## Motivation | ||
|
|
||
| This project contains extension functions for plugins that allow you to write code in the Ktor DSL style. \ | ||
| For example, you can rewrite the next code: |
There was a problem hiding this comment.
Edit: "Take the following code as an example:"
opentelemetry/README.md
Outdated
| } | ||
| ``` | ||
|
|
||
| To a more readable for `Ktor` style: |
There was a problem hiding this comment.
Edit: "Rewritten in Ktor DSL style, it looks like the following:"
opentelemetry/README.md
Outdated
|
|
||
| Let's see what we will see in the `Jaeger UI` after running the server (with Docker) and client: | ||
|
|
||
| 1. We can see two services that send opentelemetry data: `opentelemetry-ktor-sample-server` |
There was a problem hiding this comment.
I also see a third service jaeger-all-in-one (looking at http://localhost:16686/search)
There was a problem hiding this comment.
Maybe we also need one sep prior to clarify the URL the user should navigate to.
opentelemetry/README.md
Outdated
| 1. We can see two services that send opentelemetry data: `opentelemetry-ktor-sample-server` | ||
| and `opentelemetry-ktor-sample-client`: | ||
|  | ||
| 2. If we choose `opentelemetry-ktor-sample-server` service, we will see the next traces: |
There was a problem hiding this comment.
Edit: "If you select opentelemetry-ktor-sample-server service and click on Find traces, you will see a list of traces:"
opentelemetry/README.md
Outdated
|  | ||
| 2. If we choose `opentelemetry-ktor-sample-server` service, we will see the next traces: | ||
|  | ||
| 3. And if we choose one of the traces: |
There was a problem hiding this comment.
Edit: "If you click on one of those traces, you will be navigated to a screen providing detailed information about the selected trace."
KTOR-6025