GH-1444: Listener Observability Initial Commit #1500

garyrussell · 2022-09-07T17:19:12Z

expand scope to include error handler: Extending tracing scope for @RabbitListener #1287
tracing can't be used with a batch listener (multiple messages in listener call)

- expand scope to include error handler: spring-projects#1287 - tracing can't be used with a batch listener (multiple messages in listener call)

artembilan · 2022-09-07T17:21:57Z

.../main/java/org/springframework/amqp/rabbit/support/micrometer/RabbitListenerObservation.java

+
+		@Override
+		public String getContextualName() {
+			return "RabbitListener Observation";


I see this in the PropagatingSenderTracingObservationHandler:

span.name(context.getContextualName() != null ? context.getContextualName() : context.getName());

so, the plain name (in your case spring.rabbit.listener) is not going to be visible on a span.
Or there is another logic in the super interface:

/** * Get the span name. * @param context handler context * @return name for the span */ default String getSpanName(T context) { String name = context.getName(); if (StringUtils.isNotBlank(context.getContextualName())) { name = context.getContextualName(); } return SpanNameUtil.toLowerHyphen(name); }

So, we need to consult with @marcingrzejszczak to be sure where to use one or another, or both.

The standard name is the name you'll eventually see as a metric name (e.g. metrics.rabbitmq.listener - if we want to align against sth like OpenTelemetry metrics https://opentelemetry.io/docs/reference/specification/metrics/semantic_conventions/instrumentation/kafka/ cc @shakuzen). The contextual name will be translated to a span name. That could be done also in accordance with OpenTelemetry trace semantic conventions (https://opentelemetry.io/docs/reference/specification/trace/semantic_conventions/messaging/#span-name) and could be set to <destination name> <operation name> (that's why you need to have information from the context to set the name - this is why we called it contextual name)

Well, this sounds like that listenerId has to be present in this contextual name instead of the tag (or in addition to).

Why? For the standard name that would be literally metrics.rabbitmq.listener. For the contextual name you'd need to pass to the context the destination name (queue from rabbit template?) and operation name like (send / receive)

I also see that there is a Description in those metrics. And we indeed add something like that to the Timer.
But I couldn't find anything similar in the Observation API.
I may guess that this contextualName may be mapper.
But all of these are just speculations from my ignorance.

(Sorry for hijacking this PR for not related discussion 😄 )

Even if we do instrument at a different place I wonder if that will change anything for tools like Zipkin. What I mean is that if we have a RequestReply case, that implies the kind type to be CLIENT and SERVER. Zipkin never assumed that there will be a broker in between. For messaging theoretically you should always be using the SenderContext and ReceiverContext cause those use kind PRODUCER and CONSUMER that are desgined for messaging. I think we should NOT be using the RequestReply... versions cause we can break the latency visualization tools (I'm talking here about tracing only of course) cc @shakuzen @jonatan-ivanov

Yes, I also wanted to say that request-reply is an artificial feature for Messaging per se.
It is always a produce on one side and consumer on another. And then we got opposite direction for a reply process.
It might look strange for the first producer side when we block its observation waiting for a reply where ,probably, its consumer span going to look as a child...

Ok so in fact the request-reply is a fire and forget but in the other direction. So indeed we shouldn't be using the RequestReply... context but the standard Sender... and Receiver... options

I am getting a bit confused - if the contextual name must be retrieved from the context, why is there a getter on the observation itself (where there is no context).

Does this commit get us closer to what you expect?

c7d1821

The contextual name should be applied on the Convention, you should not override any methods on the context itself. The idea behind ObservationConvention is that it contains all the information about naming and tagging (keyvalues). So that if users want to change these attributes they inject their own convention instead of chanhing anything in the production code.

artembilan · 2022-09-07T17:24:28Z

...t/src/main/java/org/springframework/amqp/rabbit/support/micrometer/MessageSenderContext.java

+ * @since 3.0
+ *
+ */
+public class MessageSenderContext extends SenderContext<Message> {


I call them the same way in Spring Integration as well.
I wonder if we should think about different names.
For example RabbitSenderContext 😄 or RabbitMessageSenderContext.
Just to avoid confusion on the end-user side.

Or... since it is apparently your Pr is going to be merged first, I probably need to rename them in Spring Integration... 😉

artembilan · 2022-09-07T17:25:27Z

.../org/springframework/amqp/rabbit/support/micrometer/RabbitTemplateObservationConvention.java

+
+	@Override
+	public boolean supportsContext(Context context) {
+		return context instanceof MessageReceiverContext;


Isn't this supposed to be a SenderContext instead?

artembilan · 2022-09-07T17:26:46Z

spring-rabbit/src/test/resources/log4j2-test.xml

@@ -6,7 +6,7 @@
 		</Console>
 	</Appenders>
 	<Loggers>
-		<Logger name="org.springframework.amqp.rabbit" level="info"/>
+		<Logger name="org.springframework.amqp.rabbit" level="debug"/>


Maybe we don't need this any more?
I mean you have checked logs locally - OK: no need to increase logging for main

artembilan · 2022-09-07T17:27:21Z

build.gradle

@@ -398,6 +399,7 @@ project('spring-rabbit') {
 		testApi project(':spring-rabbit-junit')
 		testImplementation("com.willowtreeapps.assertk:assertk-jvm:$assertkVersion")
 		testImplementation "org.hibernate.validator:hibernate-validator:$hibernateValidationVersion"
+        testImplementation 'io.micrometer:micrometer-observation-test'


artembilan · 2022-09-07T17:39:51Z

spring-rabbit/src/main/java/org/springframework/amqp/rabbit/core/RabbitTemplate.java

 		// Check if commit needed
 		if (isChannelLocallyTransacted(channel)) {
 			// Transacted channel created by this template -> commit.
 			RabbitUtils.commitIfNecessary(channel);
 		}
 	}

+	protected void observeTheSend(Channel channel, Message message, boolean mandatory, String exch, String rKey,
+			Message messageToUse) {


Why do we need two Message instances over here?
Why just one messageToUse from the previous modification is not enough?

Refactor error.

artembilan · 2022-09-07T17:40:54Z

spring-rabbit/src/main/java/org/springframework/amqp/rabbit/core/RabbitTemplate.java

+			observation = RabbitTemplateObservation.TEMPLATE_OBSERVATION.observation(registry,
+						new MessageSenderContext(message))
+					.lowCardinalityKeyValue(RabbitTemplateObservation.TemplateLowCardinalityTags.BEAN_NAME.asString(),
+							this.beanName);


Cannot this be a part of MessageSenderContext and populated transparently by that RabbitTemplateObservation?

I don't see how to add key values to the context internally, addLowCardinalityKeyValue() is package-private.

I can do it via the ...Convention - stay tuned.

Yeah... Probably.
See DefaultApacheHttpClientObservationConvention for example.

Yup, you should set all keyvalues via conventions.

artembilan · 2022-09-07T19:17:24Z

.../org/springframework/amqp/rabbit/support/micrometer/RabbitListenerObservationConvention.java

+	@Override
+	public KeyValues getHighCardinalityKeyValues(RabbitMessageReceiverContext context) {
+		return this.highCardinality
+					.and(RabbitListenerObservation.ListenerLowCardinalityTags.LISTENER_ID.asString(),


I don't think we need to add this tag into high as well

oops; brain fade.

artembilan · 2022-09-07T19:18:00Z

.../org/springframework/amqp/rabbit/support/micrometer/RabbitTemplateObservationConvention.java

+	@Override
+	public KeyValues getHighCardinalityKeyValues(RabbitMessageSenderContext context) {
+		return this.highCardinality
+					.and(RabbitTemplateObservation.TemplateLowCardinalityTags.BEAN_NAME.asString(),


Missed this one, Gary.

artembilan · 2022-09-07T19:50:59Z

.../org/springframework/amqp/rabbit/support/micrometer/RabbitTemplateObservationConvention.java

+	@Override
+	public KeyValues getHighCardinalityKeyValues(RabbitMessageSenderContext context) {
+		return this.highCardinality
+					.and(RabbitTemplateObservation.TemplateLowCardinalityTags.BEAN_NAME.asString(),


Missed this one, Gary.

marcingrzejszczak · 2022-09-07T19:53:22Z

.../org/springframework/amqp/rabbit/support/micrometer/RabbitListenerObservationConvention.java

+	 * @param lowCardinality the low cardinality {@link KeyValues}.
+	 * @param highCardinality the high cardinality {@link KeyValues}.
+	 */
+	public RabbitListenerObservationConvention(@Nullable KeyValues lowCardinality,


Why do you want to allow users to add key values like this? If they want to set their own key values, they would provide their own custom convention that could simply append their key values to whatever you're setting in your default observation convention implementation

marcingrzejszczak · 2022-09-07T19:53:53Z

.../org/springframework/amqp/rabbit/support/micrometer/RabbitListenerObservationConvention.java

+
+	@Override
+	public KeyValues getLowCardinalityKeyValues(RabbitMessageReceiverContext context) {
+		return this.lowCardinality


How many listener ids can we have in a single application?

Usually only a few; probably low teens at the most.

Ok so that's indeed low-cardinality 👍

marcingrzejszczak · 2022-09-07T19:54:23Z

.../main/java/org/springframework/amqp/rabbit/support/micrometer/RabbitTemplateObservation.java

+
+		@Override
+		public String getContextualName() {
+			return "RabbitTemplate Observation";


Same comment as above

marcingrzejszczak · 2022-09-07T19:55:08Z

.../org/springframework/amqp/rabbit/support/micrometer/RabbitTemplateObservationConvention.java

+	 * @param lowCardinality the low cardinality {@link KeyValues}.
+	 * @param highCardinality the high cardinality {@link KeyValues}.
+	 */
+	public RabbitTemplateObservationConvention(@Nullable KeyValues lowCardinality,


Same comment as above in terms of additional keyvalues

...abbit/src/test/java/org/springframework/amqp/rabbit/support/micrometer/ObservationTests.java

artembilan · 2022-09-07T20:45:40Z

...src/main/java/org/springframework/amqp/rabbit/listener/AbstractMessageListenerContainer.java

 			observation = RabbitListenerObservation.LISTENER_OBSERVATION.observation(registry,
-						new RabbitMessageReceiverContext((Message) data, getListenerId()))
+						new RabbitMessageReceiverContext(message, getListenerId(),
+								message.getMessageProperties().getConsumerQueue()))


May we just do this in the target RabbitMessageReceiverContext?
We already provide a Message over there...

But it's not available in getContextualName().

Oh; sorry - I see what you mean.

artembilan · 2022-09-07T21:34:47Z

spring-rabbit/src/main/java/org/springframework/amqp/rabbit/core/RabbitTemplate.java

+
+	private boolean observationEnabled;
+
+	private RabbitTemplateObservationConvention observationConvention = new RabbitTemplateObservationConvention();


@marcingrzejszczak ,

is this what you have expected?

I thought the idea was just about a general ObservationConvention<RabbitMessageSenderContext> and that's it...

Yes, now it looks (almost) perfect! RabbitTemplateObservationConvention could be an interface that just extends the ObservationConvention<RMSC>. You would have the DefaultRTOC that would implement the RTOC.

You can check

https://github.com/spring-cloud/spring-cloud-task/blob/main/spring-cloud-task-core/src/main/java/org/springframework/cloud/task/configuration/observation/DefaultTaskObservationConvention.java#L28

https://github.com/spring-cloud/spring-cloud-task/blob/02f4581f32d061aff8e1dbb67fee2973ca1cb08d/spring-cloud-task-core/src/main/java/org/springframework/cloud/task/configuration/observation/TaskObservationConvention.java#L27

for more info

artembilan

LGTM.
It feels a bit involved with all those extra layers of enums and conventions, but I'm still learning this stuff and cannot be treated as a genuine source of opinion. 😸

marcingrzejszczak · 2022-09-07T21:49:47Z

It feels a bit involved with all those extra layers of enums and conventions, but I'm still learning this stuff and cannot be treated as a genuine source of opinion. smile_cat

Enums here are like @Configuration for Spring. They separate what you want to have as observations, how they should be named and tagged from their actual usage. An additional advantage is that due to using enums you can automatically generate documentation of your observations https://micrometer.io/docs/tracing#_documentation_building

A convention is the way to actually apply the naming and tagging. The way we designed things allow you to easily locally or globally alter that behaviour.

marcingrzejszczak · 2022-09-08T14:18:14Z

...in/java/org/springframework/amqp/rabbit/support/micrometer/RabbitMessageReceiverContext.java

+	}
+
+	@Override
+	public String getContextualName() {


You should not set it here. You should set the contextual name in the RabbitListenerObservationConvention

Does it mean that Context.getContextualName() has to be final, so we won't be able to override it?

I mean you can override everything but it was never intended to be done like that. That's because users won't be able to easily customize the contextual name.

marcingrzejszczak · 2022-09-08T14:20:21Z

...main/java/org/springframework/amqp/rabbit/support/micrometer/RabbitMessageSenderContext.java

+
+	@Override
+	public String getContextualName() {
+		return this.destination + " send";


You should not set it here. You should set the contextual name in the RabbitTemplateObservationConvention

Looks like this one is missed for a fix.

marcingrzejszczak · 2022-09-08T14:21:32Z

.../main/java/org/springframework/amqp/rabbit/support/micrometer/RabbitTemplateObservation.java

+
+		@Override
+		public String getName() {
+			return "spring.rabbit.template";


You can use the getDefaultConvention method - https://github.com/spring-cloud/spring-cloud-task/blob/main/spring-cloud-task-core/src/main/java/org/springframework/cloud/task/configuration/observation/TaskDocumentedObservation.java#L32 which gives you better customization options

Hold on. Then it means that my question about RabbitTemplateObservationConvention as a custom convention from end-user remains opened.
According that task feature we should have an extra interface and have it default impl for use-cases like this and contextual name.
The interface would be used as a point of customization from end-user, not a class as it is right now.

Yeah I modified my response here #1500 (comment)

- expand scope to include error handler: spring-projects#1287 - tracing can't be used with a batch listener (multiple messages in listener call)

…tions.

artembilan · 2022-09-13T16:15:09Z

...in/java/org/springframework/amqp/rabbit/support/micrometer/RabbitMessageReceiverContext.java

+ * {@link ReceiverContext} for {@link Message}s.
+ *
+ * @author Gary Russell
+ * @since 2.8


.../main/java/org/springframework/amqp/rabbit/support/micrometer/RabbitTemplateObservation.java

artembilan

LGTM

marcingrzejszczak · 2022-09-15T07:10:05Z

...est/java/org/springframework/amqp/rabbit/support/micrometer/ObservationIntegrationTests.java

+
+	@Override
+	public TracingSetup[] getTracingSetup() {
+		return new TracingSetup[]{ TracingSetup.IN_MEMORY_BRAVE };


You don't have to explicitly set this - if you don't have Zipkin running or Wavefront configured they will be ignored. Why have you ignored the OTel tests?

Thanks; that wasn't obvious from the docs.

https://micrometer.io/docs/tracing#_running_integration_tests

- by asserting spans that were stored without emitting them to a reporting system - against running Tanzu Observability by Wavefront instance (this option turns on when you have passed the Wavefront related configuration in the constructor - otherwise the test will be disabled) - against running Zipkin instance (this option turns on when Zipkin is running - otherwise the test will be disabled)

This looks correct to me:

So testq2 send is a child of a testq1 receive ? If that's the case then it looks good!

In some cases the name has / at the beginning - should it look like this?

Yes, the test1 receiver forwards the message to test2 which is received by the second receiver.

Yes, with RabbitMQ, the sender doesn't know the destination queue; it sends to exchange/routingKey. In this case, I am sending to the default exchange "" with a routing key equal to the queue name (which is a default binding provided by rabbitmq).

The / is not present in the Kafka PR because the sender knows the destination topic name.

artembilan

We probably also need some docs for this new stuff.

artembilan · 2022-09-15T14:21:26Z

...main/java/org/springframework/amqp/rabbit/support/micrometer/RabbitMessageSenderContext.java

+
+	@Override
+	public String getContextualName() {
+		return this.destination + " send";


Looks like this one is missed for a fix.

src/reference/asciidoc/amqp.adoc

Co-authored-by: Artem Bilan <abilan@vmware.com>

artembilan · 2022-09-15T18:41:24Z

...ringframework/amqp/rabbit/support/micrometer/DefaultRabbitListenerObservationConvention.java

+
+	@Override
+	public KeyValues getHighCardinalityKeyValues(RabbitMessageReceiverContext context) {
+		return KeyValues.empty();


Same here about default methods in the super interface.

artembilan · 2022-09-15T18:41:35Z

...ringframework/amqp/rabbit/support/micrometer/DefaultRabbitTemplateObservationConvention.java

+
+	@Override
+	public KeyValues getHighCardinalityKeyValues(RabbitMessageSenderContext context) {
+		return KeyValues.empty();


artembilan · 2022-09-15T18:42:43Z

.../main/java/org/springframework/amqp/rabbit/support/micrometer/RabbitTemplateObservation.java

+
+			@Override
+			public String asString() {
+				return "bean.name";


Same here about prefixing tags: spring-projects/spring-integration#3879 (review)

artembilan · 2022-09-15T18:44:56Z

...main/java/org/springframework/amqp/rabbit/support/micrometer/RabbitMessageSenderContext.java

+
+	@Override
+	public String getName() {
+		return "spring.rabbit.template";


I used the name directly on the DocumentedObservation impl instead of context.
Not sure what is the proper way to go though...
@marcingrzejszczak ?

Thanks

The rule of thumb should be

always try to use the names from a convention

if for some reason it doesn't make sense (you would have a lot of conventions cause you have a lot of contexts that don't differ too much) you can set one on a DocumentedObservation

marcingrzejszczak · 2022-09-16T09:00:32Z

Also we should double check that if we have 2 separate processes then the trace gets continued. It means that on the SenderContext you would need to set the remoteServiceName to be the name of the broker (e.g. rabbitmq). Then on the recipient side on the ReceiverContext you would need to se the remoteServiceName to be rabbitmq too. That way you will see in Zipkin in the dependency graph that application A is talking to a rabbitmq service that then talks to service B.

shakuzen · 2022-09-16T12:45:51Z

spring-rabbit/src/main/java/org/springframework/amqp/rabbit/core/RabbitTemplate.java

+		else {
+			observation = RabbitTemplateObservation.TEMPLATE_OBSERVATION.observation(this.observationConvention,
+					DefaultRabbitTemplateObservationConvention.INSTANCE,
+						new RabbitMessageSenderContext(message, this.beanName, exch + "/" + rKey), registry);


If we took a Supplier<Context> here instead of Context where the supplier was only called if ObservationRegistry wasn't a no-op, and therefore the Context object wouldn't be allocated in the no-op case, would that simplify things for you? I think then you wouldn't need the if-else this is contained in, right?

+1 from me as well. And I probably agree with @marcingrzejszczak as well.
Only the problem that @garyrussell is off today and next Monday is release day.
I can fix this quickly myself, but we have to be sure that this one is good to go for merging.

Thank you!

~~Or, we could just use a singleton for the No-op case.~~

Forget that, I shouldn’t comment from my iPad while not working. I see your point about them if/else.

we can go with this as is for Monday’s milestone and Polish later if you add the supplier variant.

There is one, Gary, in the DocumentedObservation:

default <T extends Observation.Context> Observation createNotStarted( @Nullable ObservationConvention<T> customConvention, @NonNull ObservationConvention<T> defaultConvention, @NonNull Supplier<T> contextSupplier, @NonNull ObservationRegistry registry) { if (registry.isNoop()) { return Observation.NOOP; } return observation(customConvention, defaultConvention, contextSupplier.get(), registry); }

I'm not sure why name is like that, but looks like it does exactly what is expected from us here.

We're working on unifying the names. Sorry about the commotion cc @ttddyy

garyrussell · 2022-09-17T20:25:45Z

@marcingrzejszczak I don't see a remoteServiceName on the ReceiverContext, only on the SenderContext.

~~Also, should the service name just be rabbitmq or should it include, for example, the ip address of the actual broker?~~

garyrussell · 2022-09-17T23:21:57Z

Forget the second comment in my last post - it wouldn't work because, with a cluster, they might be connected to different nodes.

marcingrzejszczak · 2022-09-19T06:52:50Z

@marcingrzejszczak I don't see a remoteServiceName on the ReceiverContext, only on the SenderContext.

That must be our mistake - I'll fix it (micrometer-metrics/micrometer#3419)

artembilan requested changes Sep 7, 2022

View reviewed changes

marcingrzejszczak reviewed Sep 7, 2022

View reviewed changes

artembilan requested changes Sep 7, 2022

View reviewed changes

artembilan reviewed Sep 7, 2022

View reviewed changes

artembilan approved these changes Sep 7, 2022

View reviewed changes

marcingrzejszczak reviewed Sep 8, 2022

View reviewed changes

artembilan mentioned this pull request Sep 9, 2022

Add some infrastructure for Observation spring-projects/spring-integration#3879

Merged

garyrussell added 14 commits September 13, 2022 11:52

spring-projectsGH-1444: Listener Observability Initial Commit

6f35711

- expand scope to include error handler: spring-projects#1287 - tracing can't be used with a batch listener (multiple messages in listener call)

Rename Sender/Receiver contexts and other PR review comments.

3c767f1

Rename contexts to Rabbit...; supply default KeyValues via the conven…

0886783

…tions.

Javadoc polishing.

355f5db

Don't add default KV to high-card KVs.

b39a5f9

Fix previous commit.

393ed51

Fix contextual name (receiver side).

c282d46

Fix checkstyle.

c5b2975

Polish previous commit.

9de2da2

Fix contextual name (sender side)

55f0437

Remove contextual names from observations.

3079268

Fix checkstyle.

0cf3e55

Remove customization of KeyValues from conventions.

d929b0f

Add getDefaultConvention() to observations.

b1bd023

garyrussell force-pushed the GH-1444 branch from 177aad8 to b1bd023 Compare September 13, 2022 15:52

artembilan requested changes Sep 13, 2022

View reviewed changes

Fix since 3.0.

be4be28

artembilan approved these changes Sep 14, 2022

View reviewed changes

garyrussell added 2 commits September 14, 2022 11:12

Fix Test - not sure why PR build succeeded.

f1ac117

Add Meters to ObservationTests.

03ccb33

artembilan approved these changes Sep 14, 2022

View reviewed changes

garyrussell added 3 commits September 14, 2022 12:01

Fix checkstyle.

2557794

Make INSTANCE final.

970ba6c

Add integration test.

a528d09

marcingrzejszczak reviewed Sep 15, 2022

View reviewed changes

garyrussell added 2 commits September 15, 2022 08:59

Test all available integrations.

1927f3b

Remove redundant test code.

cb6ba4c

artembilan requested changes Sep 15, 2022

View reviewed changes

Move getContextualName to conventions.

d0b758e

artembilan approved these changes Sep 15, 2022

View reviewed changes

Add docs.

0043280

artembilan requested changes Sep 15, 2022

View reviewed changes

src/reference/asciidoc/amqp.adoc Outdated Show resolved Hide resolved

Fix doc link.

599d5b7

Co-authored-by: Artem Bilan <abilan@vmware.com>

artembilan requested changes Sep 15, 2022

View reviewed changes

Remove unnecessary method overrides; make tag names more meaningful.

38589a0

artembilan approved these changes Sep 15, 2022

View reviewed changes

shakuzen reviewed Sep 16, 2022

View reviewed changes

Move getName() from contexts to conventions.

d5464ab

garyrussell added 2 commits September 19, 2022 12:08

Fix Race in Test

4ec81a1

Fix Race in Test.

2651f1c

garyrussell force-pushed the GH-1444 branch from 4269343 to 2651f1c Compare September 19, 2022 17:41

garyrussell merged commit 66e5c11 into spring-projects:main Sep 19, 2022

artembilan mentioned this pull request Oct 17, 2022

Extending tracing scope for @RabbitListener #1287

Closed


		private boolean observationEnabled;

		private RabbitTemplateObservationConvention observationConvention = new RabbitTemplateObservationConvention();

GH-1444: Listener Observability Initial Commit #1500

GH-1444: Listener Observability Initial Commit #1500

Conversation

garyrussell commented Sep 7, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcingrzejszczak Sep 8, 2022 • edited

Choose a reason for hiding this comment

artembilan left a comment

Choose a reason for hiding this comment

marcingrzejszczak commented Sep 7, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

artembilan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

artembilan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcingrzejszczak commented Sep 16, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

garyrussell Sep 16, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcingrzejszczak Sep 8, 2022 •

edited

garyrussell Sep 16, 2022 •

edited

garyrussell commented Sep 17, 2022 •

edited