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

Editing pass #331

Closed
wants to merge 1 commit into from
Closed

Editing pass #331

wants to merge 1 commit into from

Conversation

Buzzardo
Copy link
Contributor

for consistency and readability.

for consistency and readability.
jonatan-ivanov added a commit to micrometer-metrics/micrometer that referenced this pull request Jan 24, 2024
See: micrometer-metrics/micrometer-docs#331

Co-authored-by: Jay Bryant <jbryant@vmware.com>

TIP: Gauges are useful for monitoring things with natural upper bounds. We do not recommend using a gauge to monitor things like request count, as they can grow without bound for the duration of an application instance's life.

TIP: Never gauge something you can count with a `Counter`!

Micrometer takes the stance that gauges should be sampled and not be set, so there is no information about what might have occurred between samples. Any intermediate values set on a gauge are lost by the time the gauge value is reported to a metrics backend, so there is little value in setting those intermediate values in the first place.

Think of a `Gauge` as a "`heisen-gauge`": a meter that changes only when it is observed. Every other meter type accumulates intermediate counts toward the point where the data is sent to the metrics backend.
Think of a `Gauge` as a "`heisenberg-gauge`": a meter that changes only when it is observed. Every other meter type accumulates intermediate counts toward the point where the data is sent to the metrics backend.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this was intentionally paraphrasing the word heisenbug, I would keep it as is unless this is problematic.

@@ -20,9 +20,11 @@ Micrometer contains a core module with an instrumentation https://en.wikipedia.o
|Server-side

|AppOptics, Atlas, Azure Monitor, Datadog, Dynatrace, Elastic, Graphite, Ganglia, Humio, Influx, JMX, Kairos, New Relic, all StatsD flavors, SignalFx
|Prometheus, Wavefront footnote:[As of 1.2.0, Micrometer sends cumulative values to Wavefront.]
|Prometheus, Wavefront
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

footnote is a markdown feature and it places a footnote to the bottom with a link.


The `micrometer-core` modules contains a `@Timed` annotation that frameworks can use to add timing support to either specific types of methods such as those serving web request endpoints or, more generally, to all methods.
The `micrometer-core` modules contain a `@Timed` annotation that frameworks can use to add timing support to either specific types of methods such as those serving web request endpoints or, more generally, to all methods.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be:

The micrometer-core module contains

@@ -51,8 +51,8 @@ This section serves as a quick start to rendering useful representations in AppO
The AppOptics implementation of `Timer` produces three fields in AppOptics:

* `sum`: Rate of calls per second.
* `count`: Rate of total time per second.
* `max`: A sliding window maximum amount recorded.
* `count`: Rate of total time (updated every second).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there are more issues here, sum is the rate of total time/sec while count is the rate of number of calls/sec.

@@ -8,7 +8,7 @@ https://aws.amazon.com/cloudwatch/[Amazon CloudWatch] is a dimensional time-seri

include::install.adoc[]

NOTE: The `micrometer-registry-cloudwatch2` module uses AWS SDK v2. `micrometer-registry-cloudwatch` is for AWS SDK v1.
NOTE: The `micrometer-registry-cloudwatch2` module uses AWS SDK v2. `micrometer-registry-cloudwatch` is for version 1 of the AWS SDK.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name of the component is AWS SDK v1, I think I would keep it that way.

Existing setups will continue to work when updating to newer versions of Micrometer.
The reported metrics will be assigned to https://www.dynatrace.com/support/help/dynatrace-api/environment-api/topology-and-smartscape/custom-device-api/report-custom-device-metric-via-rest-api/[custom devices] in Dynatrace.
Existing setups continue to work when updating to newer versions of Micrometer.
The reported metrics are assigned to https://www.dynatrace.com/support/help/dynatrace-api/environment-api/topology-and-smartscape/Custom-Eevice-api/report-Custom-Eevice-metric-via-rest-api/[Custom Eevices] in Dynatrace.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The url change is not needed + "Custom Devices".

@@ -54,8 +54,8 @@ This section serves as a quick start to rendering useful representations in Humi
The Humio implementation of `Timer` produces four fields in Humio:

* `sum`: Rate of calls per second.
* `count`: Rate of total time per second.
* `max`: A sliding window maximum amount recorded.
* `count`: Rate of total time (updated each second).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has the same issue as AppOptics: sum is the rate of total time/sec while count is the rate of number of calls/sec.

@@ -12,7 +12,7 @@ You can collect metrics from `Jetty` by adding `JettyConnectionMetrics` as follo

Micrometer also supports binding metrics to `Jersey` through `ApplicationEventListener`.

You can collect metrics from `Jersey` by adding `MetricsApplicationEventListener` as follows:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if we remove the code block from Jetty, we should remove it from Jersey too.


[[instrumentation_of_reactive_libraries_before_reactor_3_5_3]]
=== Before Reactor 3.5.3

The preferred way of propagating elements through the Flux using Reactor is not via ``ThreadLocal``s but through Reactor Context. Reactor however gives you two operators, `tap()` and `handle()` where, if https://micrometer.io/docs/contextPropagation[Micrometer Context Propagation] library is on the classpath, it will set thread local values for you.
The preferred way of propagating elements through the Flux by using Reactor is not through `ThreadLocal` instancess but through Reactor Context. Reactor, however, gives you two operators: `tap()` and `handle()`.With these two operators, if the https://micrometer.io/docs/contextPropagation[Micrometer Context Propagation] library is on the classpath, it sets the thread local values for you.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instances instead of instancess and missing space: ". With"

@@ -54,4 +54,4 @@ https://github.com/redisson/redisson/wiki/16.-Observability#162-tracing[Docs]
| Spring WebFlux | https://docs.spring.io/spring-framework/reference/integration/observability.html[Docs]
|===

If your project is instrumented using Micrometer Observation, and it's not listed in the table above, https://github.com/micrometer-metrics/micrometer-docs/edit/main/src/docs/observation/observation-projects.adoc[please file a PR] to our documentation! If you want to instrument your project and need our help just mention us in your issue - https://github.com/shakuzen/[@shakuzen], https://github.com/jonatan-ivanov/[@jonatan-ivanov], https://github.com/marcingrzejszczak/[@marcingrzejszczak].
If your project is instrumented witj Micrometer Observation and is not listed in the table, https://github.com/micrometer-metrics/micrometer-docs/edit/main/src/docs/observation/observation-projects.adoc[please file a PR] to our documentation! If you want to instrument your project and need our help, mention us in your issue - https://github.com/shakuzen/[@shakuzen], https://github.com/jonatan-ivanov/[@jonatan-ivanov], and https://github.com/marcingrzejszczak/[@marcingrzejszczak].
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with

jonatan-ivanov added a commit to micrometer-metrics/context-propagation that referenced this pull request Jan 24, 2024
It seems editing pass from the old docs
micrometer-metrics/micrometer-docs#331
were already migrated in gh-143
jonatan-ivanov added a commit to micrometer-metrics/micrometer-docs-generator that referenced this pull request Jan 24, 2024

.ObservationThreadLocalAccessor
[source,java,subs=+attributes]
-----
include::../../../samples/src/test/java/io/micrometer/docs/context/ObservationThreadLocalAccessor.java[tags=accessor,indent=0]
-----

Below you can find an example of how to store and restore thread local values via `ThreadLocalAccessor`, `ContextSnapshot` and `ContextRegistry`.
The following example whows how to store and restore thread local values via `ThreadLocalAccessor`, `ContextSnapshot`, and `ContextRegistry`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shows


.build.gradle
[source,groovy,subs=+attributes]
-----
include::../../../samples/src/test/resources/docs-generator-build.gradle[indent=0,tag=main]
-----

Running these tasks would lead to generation of adoc files similar to these ones.
Running these tasks would lead to generation of adoc files similar to these :
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra space before :

| Handlebars template file location. This can be a path in the classpath or file system. +
e.g. `templates/metrics.adoc.hbs`, `/home/foo/bar.hbs`
| Handlebars template file location. This can be a path in the classpath or file system, +
such as `templates/metrics.adoc.hbs` or `/home/example/bar.hbs`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm gong to fix these extra spaces (after "such as").


[source,java,subs=+attributes]
-----
include::../../../samples/src/test/java/io/micrometer/docs/tracing/TracingApiTests.java[tags=manual_span_creation,indent=0]
-----

Below you can see how to continue a span in a new thread, that was started in another thread.
The following example shows how to continue a span that was started in another thread in a new thread:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To me the previous version of the second part of this makes more sense:

The following example shows how to continue a span in a new thread that was started in another thread:


[source,java,subs=+attributes]
-----
include::../../../samples/src/test/java/io/micrometer/docs/tracing/TracingConfiguringTests.java[tags=handler_configuration,indent=0]
-----

You can also use a shorter version to perform measurements via the `observe` method.
You can also use a shorter version to perform measurements by using the `observe` method"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

: instead of "

jonatan-ivanov added a commit to micrometer-metrics/tracing that referenced this pull request Jan 24, 2024
@jonatan-ivanov
Copy link
Member

I moved the changes in this PR:

My review can be ignored, I just recorded the changes I made.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants