Skip to content

Commit 18530c8

Browse files
committed
Add PhasedObservation
Observation itself does not protect against start and stop being called multiple times. This commit aligns all observation instances to instead use an implementation that does have these guards in place. Closes gh-14082
1 parent 6a29f7e commit 18530c8

File tree

1 file changed

+121
-76
lines changed

1 file changed

+121
-76
lines changed

web/src/main/java/org/springframework/security/web/server/ObservationWebFilterChainDecorator.java

Lines changed: 121 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -258,21 +258,21 @@ default Observation after() {
258258

259259
class SimpleAroundWebFilterObservation implements AroundWebFilterObservation {
260260

261-
private final ObservationReference before;
261+
private final PhasedObservation before;
262262

263-
private final ObservationReference after;
263+
private final PhasedObservation after;
264264

265-
private final AtomicReference<ObservationReference> currentObservation = new AtomicReference<>(
266-
ObservationReference.NOOP);
265+
private final AtomicReference<PhasedObservation> currentObservation = new AtomicReference<>(
266+
PhasedObservation.NOOP);
267267

268268
SimpleAroundWebFilterObservation(Observation before, Observation after) {
269-
this.before = new ObservationReference(before);
270-
this.after = new ObservationReference(after);
269+
this.before = new PhasedObservation(before);
270+
this.after = new PhasedObservation(after);
271271
}
272272

273273
@Override
274274
public Observation start() {
275-
if (this.currentObservation.compareAndSet(ObservationReference.NOOP, this.before)) {
275+
if (this.currentObservation.compareAndSet(PhasedObservation.NOOP, this.before)) {
276276
this.before.start();
277277
return this.before.observation;
278278
}
@@ -389,73 +389,6 @@ public String toString() {
389389
return this.currentObservation.get().observation.toString();
390390
}
391391

392-
private static final class ObservationReference {
393-
394-
private static final ObservationReference NOOP = new ObservationReference(Observation.NOOP);
395-
396-
private final Lock lock = new ReentrantLock();
397-
398-
private final AtomicInteger state = new AtomicInteger(0);
399-
400-
private final Observation observation;
401-
402-
private ObservationReference(Observation observation) {
403-
this.observation = observation;
404-
}
405-
406-
private void start() {
407-
try {
408-
this.lock.lock();
409-
if (this.state.compareAndSet(0, 1)) {
410-
this.observation.start();
411-
}
412-
}
413-
finally {
414-
this.lock.unlock();
415-
}
416-
}
417-
418-
private void error(Throwable ex) {
419-
try {
420-
this.lock.lock();
421-
if (this.state.get() == 1) {
422-
this.observation.error(ex);
423-
}
424-
}
425-
finally {
426-
this.lock.unlock();
427-
}
428-
}
429-
430-
private void stop() {
431-
try {
432-
this.lock.lock();
433-
if (this.state.compareAndSet(1, 2)) {
434-
this.observation.stop();
435-
}
436-
}
437-
finally {
438-
this.lock.unlock();
439-
}
440-
}
441-
442-
private void close() {
443-
try {
444-
this.lock.lock();
445-
if (this.state.compareAndSet(1, 3)) {
446-
this.observation.stop();
447-
}
448-
else {
449-
this.state.set(3);
450-
}
451-
}
452-
finally {
453-
this.lock.unlock();
454-
}
455-
}
456-
457-
}
458-
459392
}
460393

461394
}
@@ -537,10 +470,10 @@ default WebFilterChain wrap(WebFilterChain chain) {
537470

538471
class SimpleWebFilterObservation implements WebFilterObservation {
539472

540-
private final Observation observation;
473+
private final PhasedObservation observation;
541474

542475
SimpleWebFilterObservation(Observation observation) {
543-
this.observation = observation;
476+
this.observation = new PhasedObservation(observation);
544477
}
545478

546479
@Override
@@ -728,4 +661,116 @@ public boolean supportsContext(Observation.Context context) {
728661

729662
}
730663

664+
private static final class PhasedObservation implements Observation {
665+
666+
private static final PhasedObservation NOOP = new PhasedObservation(Observation.NOOP);
667+
668+
private final Lock lock = new ReentrantLock();
669+
670+
private final AtomicInteger phase = new AtomicInteger(0);
671+
672+
private final Observation observation;
673+
674+
private PhasedObservation(Observation observation) {
675+
this.observation = observation;
676+
}
677+
678+
@Override
679+
public Observation contextualName(String contextualName) {
680+
return this.observation.contextualName(contextualName);
681+
}
682+
683+
@Override
684+
public Observation parentObservation(Observation parentObservation) {
685+
return this.observation.parentObservation(parentObservation);
686+
}
687+
688+
@Override
689+
public Observation lowCardinalityKeyValue(KeyValue keyValue) {
690+
return this.observation.lowCardinalityKeyValue(keyValue);
691+
}
692+
693+
@Override
694+
public Observation highCardinalityKeyValue(KeyValue keyValue) {
695+
return this.observation.highCardinalityKeyValue(keyValue);
696+
}
697+
698+
@Override
699+
public Observation observationConvention(ObservationConvention<?> observationConvention) {
700+
return this.observation.observationConvention(observationConvention);
701+
}
702+
703+
@Override
704+
public Observation event(Event event) {
705+
return this.observation.event(event);
706+
}
707+
708+
@Override
709+
public Context getContext() {
710+
return this.observation.getContext();
711+
}
712+
713+
@Override
714+
public Scope openScope() {
715+
return this.observation.openScope();
716+
}
717+
718+
@Override
719+
public PhasedObservation start() {
720+
try {
721+
this.lock.lock();
722+
if (this.phase.compareAndSet(0, 1)) {
723+
this.observation.start();
724+
}
725+
}
726+
finally {
727+
this.lock.unlock();
728+
}
729+
return this;
730+
}
731+
732+
@Override
733+
public PhasedObservation error(Throwable ex) {
734+
try {
735+
this.lock.lock();
736+
if (this.phase.get() == 1) {
737+
this.observation.error(ex);
738+
}
739+
}
740+
finally {
741+
this.lock.unlock();
742+
}
743+
return this;
744+
}
745+
746+
@Override
747+
public void stop() {
748+
try {
749+
this.lock.lock();
750+
if (this.phase.compareAndSet(1, 2)) {
751+
this.observation.stop();
752+
}
753+
}
754+
finally {
755+
this.lock.unlock();
756+
}
757+
}
758+
759+
void close() {
760+
try {
761+
this.lock.lock();
762+
if (this.phase.compareAndSet(1, 3)) {
763+
this.observation.stop();
764+
}
765+
else {
766+
this.phase.set(3);
767+
}
768+
}
769+
finally {
770+
this.lock.unlock();
771+
}
772+
}
773+
774+
}
775+
731776
}

0 commit comments

Comments
 (0)