Skip to content
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

Quite some issues in the Java manual instrumentation doc #3765

Open
brightzheng100 opened this issue Jan 12, 2024 · 3 comments
Open

Quite some issues in the Java manual instrumentation doc #3765

brightzheng100 opened this issue Jan 12, 2024 · 3 comments
Labels
bug Something isn't working discussion Input from everyone is helpful to drive this forward sig:java

Comments

@brightzheng100
Copy link
Contributor

What needs to be changed? Describe the update that is required.

When trying to quickly walk through the Java manual instrumentation doc here, I found a series of issues even I haven't completely walked it through yet:

  1. Since it's with Spring Boot 3.x, we should explicitly mention that JDK v17 is the minimum version required. I'd highly suggest to have a dedicated "prerequisites" section so that the reader knows what must be ready before walking through the tutorial.
  2. The example dice app uses mavenCentral(), and defines io.opentelemetry:opentelemetry-semconv:1.34.1-alpha as one of the dependencies, but actually the latest version in Maven Central of that lib is 1.30.1-alpha. Luckily, if we follow carefully we'd realize that it's just some typo thingy as the later section will use the right libs.
  3. Quite some small code errors like the ";" is missing.
  4. The later part of the tutorial is talking something completely different without properly using the example dice app -- if the example app is not good, build another one so the tutorial sticks to it throughout the process.

In short, if it's a reference doc, focus on the capabilities / features and how to use them; if it's a tutorial with a given example, the example must be suitable to demonstrate most, if not all capabilities / features, and then make it a complete workable project.

@brightzheng100 brightzheng100 added the bug Something isn't working label Jan 12, 2024
@svrnm
Copy link
Member

svrnm commented Jan 12, 2024

Thanks for raising this issue, @brightzheng100!

What needs to be changed? Describe the update that is required.
1. Since it's with Spring Boot 3.x, we should explicitly mention that JDK v17 is the minimum version required. I'd highly suggest to have a dedicated "prerequisites" section so that the reader knows what must be ready before walking through the tutorial.

good point. I assume the same applies for the getting started then.

  1. The example dice app uses mavenCentral(), and defines io.opentelemetry:opentelemetry-semconv:1.34.1-alpha as one of the dependencies, but actually the latest version in Maven Central of that lib is 1.30.1-alpha. Luckily, if we follow carefully we'd realize that it's just some typo thingy as the later section will use the right libs.

Yes, that's a minor thing that should be fixable (there's a param vers.otel where it should be params vers.semconv)

  1. Quite some small code errors like the ";" is missing.

If you know the exact issues, feel free to fix them via a PR.

  1. The later part of the tutorial is talking something completely different without properly using the example dice app -- if the example app is not good, build another one so the tutorial sticks to it throughout the process.

See #3229 for this. This is a mix of old docs (no roll the dice app) and new docs (with roll the dice app). So far nobody took the time and effort to fix this.

In short, if it's a reference doc, focus on the capabilities / features and how to use them; if it's a tutorial with a given example, the example must be suitable to demonstrate most, if not all capabilities / features, and then make it a complete workable project.

It's a reference documentation that provides you with an example throughout the documentation. That's at least the goal. Unfortunately the docs team is very limited in bandwidth to complete this. So we appreciate any help to close those gaps.

@open-telemetry/java-approvers PTAL

@pegasas
Copy link
Contributor

pegasas commented Jan 31, 2024

  1. Since it's with Spring Boot 3.x, we should explicitly mention that JDK v17 is the minimum version required. I'd highly suggest to have a dedicated "prerequisites" section so that the reader knows what must be ready before walking through the tutorial.

I am +1 for this question. raise a PR for fix this.

  1. The example dice app uses mavenCentral(), and defines io.opentelemetry:opentelemetry-semconv:1.34.1-alpha as one of the dependencies, but actually the latest version in Maven Central of that lib is 1.30.1-alpha. Luckily, if we follow carefully we'd realize that it's just some typo thingy as the later section will use the right libs.

I am -1 for this.
here's an example for https://nightlies.apache.org/flink/flink-docs-master/docs/dev/table/jdbcdriver/, 1.19-SNAPSHOT also not release in maven central.
maybe we shoud only mension {VERSION} here.

  1. Quite some small code errors like the ";" is missing.
  2. The later part of the tutorial is talking something completely different without properly using the example dice app -- if the example app is not good, build another one so the tutorial sticks to it throughout the process.
    In short, if it's a reference doc, focus on the capabilities / features and how to use them; if it's a tutorial with a given example, the example must be suitable to demonstrate most, if not all capabilities / features, and then make it a complete workable project.

I am -1 for 4. due to opentelemetry target to collabrate logging, trace, metrics in distributed environment. it will be much easier if we have a minimal web app there.

@svrnm
Copy link
Member

svrnm commented Feb 1, 2024

  1. Since it's with Spring Boot 3.x, we should explicitly mention that JDK v17 is the minimum version required. I'd highly suggest to have a dedicated "prerequisites" section so that the reader knows what must be ready before walking through the tutorial.

I am +1 for this question. raise a PR for fix this.

  1. The example dice app uses mavenCentral(), and defines io.opentelemetry:opentelemetry-semconv:1.34.1-alpha as one of the dependencies, but actually the latest version in Maven Central of that lib is 1.30.1-alpha. Luckily, if we follow carefully we'd realize that it's just some typo thingy as the later section will use the right libs.

I am -1 for this. here's an example for nightlies.apache.org/flink/flink-docs-master/docs/dev/table/jdbcdriver, 1.19-SNAPSHOT also not release in maven central. maybe we shoud only mension {VERSION} here.

I think the issue here is that the wrong version is applied:

    implementation("io.opentelemetry:opentelemetry-semconv:{{% param vers.otel %}}-alpha");

should become

    implementation("io.opentelemetry:opentelemetry-semconv:{{% param vers.semconv %}}-alpha");
  1. Quite some small code errors like the ";" is missing.
  2. The later part of the tutorial is talking something completely different without properly using the example dice app -- if the example app is not good, build another one so the tutorial sticks to it throughout the process.
    In short, if it's a reference doc, focus on the capabilities / features and how to use them; if it's a tutorial with a given example, the example must be suitable to demonstrate most, if not all capabilities / features, and then make it a complete workable project.

I am -1 for 4. due to opentelemetry target to collabrate logging, trace, metrics in distributed environment. it will be much easier if we have a minimal web app there.

Yes, this docs just need some love and attention, as outlined in #3229, similar to #3853, Java needs a continued flow for metrics

@theletterf theletterf added the discussion Input from everyone is helpful to drive this forward label Apr 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working discussion Input from everyone is helpful to drive this forward sig:java
Projects
None yet
Development

No branches or pull requests

4 participants