Skip to content

Commit 4b6dcd6

Browse files
matttproudmatttproud
committed
Merge pull request prometheus#33 from prometheus/feature/documentation/fix-partial-notes
Fix notes about thread safety and mutability.
2 parents ebd19c3 + 1ea0293 commit 4b6dcd6

5 files changed

Lines changed: 357 additions & 65 deletions

File tree

‎client/src/main/java/io/prometheus/client/metrics/Counter.java‎

Lines changed: 81 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,11 @@
1616

1717
import com.google.common.base.Optional;
1818
import com.google.common.util.concurrent.AtomicDouble;
19-
import com.google.gson.*;
19+
import com.google.gson.JsonArray;
20+
import com.google.gson.JsonElement;
21+
import com.google.gson.JsonObject;
22+
import com.google.gson.JsonSerializationContext;
23+
import com.google.gson.JsonSerializer;
2024
import io.prometheus.client.Metrics;
2125
import io.prometheus.client.utility.labels.Reserved;
2226

@@ -249,17 +253,77 @@ public Counter build() {
249253
* {@link Counter.Child}.
250254
* </p>
251255
*
256+
* <p>
257+
* <em>Warning:</em> All mutations to {@link Partial} are retained. You should <em>not</em>
258+
* share {@link Partial} between distinct label sets unless you have a parent
259+
* {@link Partial} that you {@link io.prometheus.client.metrics.Counter.Partial#clone()}.
260+
* </p>
261+
*
262+
* <p>
263+
* In this example below, we have both a race condition with a nasty outcome that
264+
* unformedMetric is mutated in both threads and that it is an undefined behavior, which
265+
* {@code data-type} label pair setting wins.
266+
* </p>
267+
*
268+
* <pre>
269+
* {@code
270+
* Counter.Partial unformedMetric = …;
271+
*
272+
* new Thread() {
273+
* public void run() {
274+
* unformedMetric.labelPair("system", "cache");
275+
* .labelPair("data-type", "user-profile"); // Difference
276+
* .apply()
277+
* .increment();
278+
* }
279+
* }.start();
280+
*
281+
* new Thread() {
282+
* public void run() {
283+
* unformedMetric.labelPair("system", "cache");
284+
* .labelPair("data-type", "avatar"); // Difference
285+
* .apply()
286+
* .increment();
287+
* }
288+
* }.start();
289+
* }
290+
* </pre>
291+
*
292+
* <p>
293+
* The following is preferable and {@link ThreadSafe}:
294+
* </p>
295+
* <pre>
296+
* {@code
297+
* Counter.Partial unformedMetric = …;
298+
*
299+
* new Thread() {
300+
* public void run() {
301+
* Counter.Partial local = unformedMetric.clone(); // Safe step!
302+
*
303+
* local.labelPair("system", "cache");
304+
* .labelPair("data-type", "user-profile"); // Difference
305+
* .apply()
306+
* .increment();
307+
* }
308+
* }.start();
309+
*
310+
* new Thread() {
311+
* public void run() {
312+
* Counter.Partial local = unformedMetric.clone(); // Safe step!
313+
*
314+
* local.labelPair("system", "cache");
315+
* .labelPair("data-type", "avatar"); // Difference
316+
* .apply()
317+
* .increment();
318+
* }
319+
* }.start();
320+
* }
321+
* </pre>
322+
*
252323
* @see Metric.Partial
253324
*/
254325
@NotThreadSafe
255326
public class Partial extends Metric.Partial {
256-
/**
257-
* <p>
258-
* Add a label-value pair to this metric.
259-
* </p>
260-
*
261-
* @see Metric.Partial#labelPair(String, String)
262-
*/
263327
@Override
264328
public Partial labelPair(String labelName, String labelValue) {
265329
return (Partial) baseLabelPair(labelName, labelValue);
@@ -277,8 +341,8 @@ protected Counter.Child newChild() {
277341

278342
/**
279343
* <p>
280-
* Finalize this child to perform mutations under this set of label-value
281-
* pairs.
344+
* Finalize this child under this set of label-value pairs and create a {@link Counter.Child}
345+
* to mutate.
282346
* </p>
283347
*
284348
* @see io.prometheus.client.metrics.Metric.Partial#apply()
@@ -295,6 +359,13 @@ public Counter.Child apply() {
295359
* dimensions.
296360
* </p>
297361
*
362+
* <p>
363+
* <em>Warning:</em> Do not hold onto a reference of a {@link Child} if you
364+
* ever use the {@link #resetAll()}. If you want to hold onto a concrete
365+
* instance, please hold onto a {@link io.prometheus.client.metrics.Counter.Partial} and use
366+
* {@link io.prometheus.client.metrics.Counter.Partial#apply()}.
367+
* </p>
368+
*
298369
* @see Metric.Child
299370
*/
300371
@ThreadSafe

‎client/src/main/java/io/prometheus/client/metrics/Gauge.java‎

Lines changed: 80 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,11 @@
1616

1717
import com.google.common.base.Optional;
1818
import com.google.common.util.concurrent.AtomicDouble;
19-
import com.google.gson.*;
19+
import com.google.gson.JsonArray;
20+
import com.google.gson.JsonElement;
21+
import com.google.gson.JsonObject;
22+
import com.google.gson.JsonSerializationContext;
23+
import com.google.gson.JsonSerializer;
2024
import io.prometheus.client.Metrics;
2125
import io.prometheus.client.utility.labels.Reserved;
2226

@@ -254,20 +258,77 @@ public Gauge build() {
254258
* {@link Gauge.Child}.
255259
* </p>
256260
*
261+
* <p>
262+
* <em>Warning:</em> All mutations to {@link Partial} are retained. You should <em>not</em>
263+
* share {@link Partial} between distinct label sets unless you have a parent
264+
* {@link Partial} that you {@link io.prometheus.client.metrics.Gauge.Partial#clone()}.
265+
* </p>
266+
*
267+
* <p>
268+
* In this example below, we have both a race condition with a nasty outcome that
269+
* unformedMetric is mutated in both threads and that it is an undefined behavior, which
270+
* {@code data-type} label pair setting wins.
271+
* </p>
272+
*
273+
* <pre>
274+
* {@code
275+
* Gauge.Partial unformedMetric = …;
276+
*
277+
* new Thread() {
278+
* public void run() {
279+
* unformedMetric.labelPair("system", "cache");
280+
* .labelPair("data-type", "user-profile"); // Difference
281+
* .apply()
282+
* .set(1);
283+
* }
284+
* }.start();
285+
*
286+
* new Thread() {
287+
* public void run() {
288+
* unformedMetric.labelPair("system", "cache");
289+
* .labelPair("data-type", "avatar"); // Difference
290+
* .apply()
291+
* .set(15);
292+
* }
293+
* }.start();
294+
* }
295+
* </pre>
296+
*
297+
* <p>
298+
* The following is preferable and {@link ThreadSafe}:
299+
* </p>
300+
* <pre>
301+
* {@code
302+
* Gauge.Partial unformedMetric = …;
303+
*
304+
* new Thread() {
305+
* public void run() {
306+
* Gauge.Partial local = unformedMetric.clone(); // Safe step!
307+
*
308+
* local.labelPair("system", "cache");
309+
* .labelPair("data-type", "user-profile"); // Difference
310+
* .apply()
311+
* .set(5);
312+
* }
313+
* }.start();
314+
*
315+
* new Thread() {
316+
* public void run() {
317+
* Gauge.Partial local = unformedMetric.clone(); // Safe step!
318+
*
319+
* local.labelPair("system", "cache");
320+
* .labelPair("data-type", "avatar"); // Difference
321+
* .apply()
322+
* .set(15);
323+
* }
324+
* }.start();
325+
* }
326+
* </pre>
327+
*
257328
* @see Metric.Partial
258329
*/
259330
@NotThreadSafe
260331
public class Partial extends Metric.Partial {
261-
/**
262-
* <p>
263-
* <em>Warning:</em> Do not hold onto a reference of a {@link Partial} if
264-
* you ever use the {@link #resetAll()} or
265-
* {@link io.prometheus.client.metrics.Metric.Child#reset()} tools. This
266-
* will be fixed in a follow-up release.
267-
* </p>
268-
*
269-
* @see Metric.Partial#labelPair(String, String)
270-
*/
271332
@Override
272333
public Partial labelPair(String labelName, String labelValue) {
273334
return (Partial) baseLabelPair(labelName, labelValue);
@@ -302,6 +363,13 @@ public Gauge.Child apply() {
302363
* A concrete instance of {@link Gauge} for a unique set of label dimensions.
303364
* </p>
304365
*
366+
* <p>
367+
* <em>Warning:</em> Do not hold onto a reference of a {@link Child} if you
368+
* ever use the {@link #resetAll()}. If you want to hold onto a concrete
369+
* instance, please hold onto a {@link io.prometheus.client.metrics.Gauge.Partial} and use
370+
* {@link io.prometheus.client.metrics.Gauge.Partial#apply()}.
371+
* </p>
372+
*
305373
* @see Metric.Child
306374
*/
307375
@ThreadSafe
@@ -332,7 +400,7 @@ public void reset() {
332400

333401
/**
334402
* <p>
335-
* Used to serialize {@link Gauge} instances for {@link Gson}.
403+
* Used to serialize {@link Gauge} instances for {@link com.google.gson.Gson}.
336404
* </p>
337405
*/
338406
@Deprecated

‎client/src/main/java/io/prometheus/client/metrics/Metric.java‎

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,18 @@
1818
import com.google.common.base.Preconditions;
1919
import io.prometheus.client.Metrics;
2020
import io.prometheus.client.Prometheus;
21+
import net.jcip.annotations.Immutable;
2122
import net.jcip.annotations.NotThreadSafe;
2223
import net.jcip.annotations.ThreadSafe;
2324

24-
import java.util.*;
25+
import java.util.ArrayList;
26+
import java.util.Arrays;
27+
import java.util.Collections;
28+
import java.util.HashMap;
29+
import java.util.HashSet;
30+
import java.util.List;
31+
import java.util.Map;
32+
import java.util.TreeMap;
2533
import java.util.concurrent.ConcurrentHashMap;
2634

2735
/**
@@ -107,7 +115,21 @@ private void disbandChildren() {
107115
children.clear();
108116
}
109117

118+
119+
/**
120+
* <p>
121+
* A {@link Builder} is used to create a {@link Metric}'s descriptor, which describes everything
122+
* about the metric: name, required label dimensions (i.e., metric cardinality), docstring, etc.
123+
* </p>
124+
*
125+
* <p>
126+
* {@link Builder} are {@link Immutable} and {@link ThreadSafe}, so you can use them to create
127+
* metric templates when there is redundancy between the definitions. All methods return a new
128+
* {@link Builder} instance.
129+
* </p>
130+
*/
110131
@ThreadSafe
132+
@Immutable
111133
public static interface Builder<B, M> {
112134
/**
113135
* <p>
@@ -647,9 +669,7 @@ public int hashCode() {
647669
*
648670
* <p>
649671
* <em>Warning:</em> Do not hold onto a reference of a {@link Child} if you
650-
* ever use the {@link #resetAll()} or
651-
* {@link io.prometheus.client.metrics.Metric.Child#reset()} tools. This will
652-
* be fixed in a follow-up release.
672+
* ever use the {@link #resetAll()}.
653673
* </p>
654674
*/
655675
public static interface Child {

0 commit comments

Comments
 (0)