feat: Create Logs SDK LoggerProvider #1517

kaylareopelle · 2023-08-25T19:12:07Z

Fulfills the Logs SDK specs for Logger Provider

The Logger Provider implementation is drawn mainly from the Tracer Provider implementation, as they have very similar specs.

If merged, this PR would fulfill the following elements on the spec-compliance-matrix for Logs

LoggerProvider.GetLogger
LoggerProvider.GetLogger accepts attributes
LoggerProvider.Shutdown
LoggerProvider.ForceFlush

Here are some other implementations of LoggerProvider for comparison:

Resolves #1483

logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb

kaylareopelle · 2023-08-25T19:20:53Z

logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb

+        rescue StandardError => e
+          OpenTelemetry.handle_error(exception: e, message: FORCE_FLUSH_ERROR)
+          Export::FAILURE
+        end


There is not a rescue in the TracerProvider#force_flush method, but the method documentation states "Export::FAILURE should be returned if a non-specific failure occurred." I'm assuming a non-specific failure would be any StandardError instance.

Is there a condition to return Export::FAILURE on non-specific errors in TracerProvider#force_flush that I'm missing?

Similar documentation exists for the LoggerProvider/TracerProvider#shutdown methods. If we choose to add a rescue here, should the #shutdown methods also have a similar rescue?

We might be assuming that processors will handle their own errors, but that might be optimistic. I would be fine with rescuing from StandardError in force_flush and shutdown across the board. I'll let others chime in if they have opinions.

kaylareopelle · 2023-08-28T22:40:48Z

logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb

+        #   {LogRecordProcessor} to add to this LoggerProvider.
+        def add_log_record_processor(log_record_processor)
+          @mutex.synchronize do
+            @log_record_processors = log_record_processors.dup.push(log_record_processor)


This is copied from TracerProvider#add_span_processor. @fallwith and I were chatting about this LOC. He was asking about the use of .dup.push here.

Does dup.push provide atomicity in a way that the push method alone couldn't deliver? Or is there a different reason dup.push was used?

I'm uncertain, but @fbogsany probably knows. I think it has to do with mutating the processors array. Looking at TracerProvider a span gets a reference to processors on start. If a span processor is added after a span is started, and the processors array is mutated, a span will see a processor that wasn't there when it was started. I'm not sure if this would happen in the real world, or if this is what we're protecting against, but that is a reason why we might prefer dup.push over push.

Yes, that's exactly the reason - each SDK Span instance has a reference to the list of span processors, and that list should not change between the span calling on_start and on_finish. IDK if that applies in your case - I need to read the rest of the PR.

It's worth considering whether it should be valid to add log record processors after @stopped. In the TracerProvider, we warn and don't allow that.

logs_sdk/lib/opentelemetry/sdk/logs/log_record_processor.rb

kaylareopelle · 2023-10-02T18:57:01Z

logs_sdk/lib/opentelemetry/sdk/logs/export.rb

+# Copyright The OpenTelemetry Authors
+#
+# SPDX-License-Identifier: Apache-2.0
+


This matches the pattern in OpenTelemetry::SDK::Trace::Export

The ExportError class is not present because it was not needed for the LoggerProvider. It will be added once it's used in subsequent PRs.

kaylareopelle · 2023-10-02T18:57:51Z

logs_sdk/lib/opentelemetry/sdk/logs/log_record_processor.rb

@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+


This file is a boilerplate/placeholder so the LogRecordProcessor methods can be called. They will be implemented in a later PR.

kaylareopelle · 2023-10-02T18:58:54Z

logs_sdk/lib/opentelemetry/sdk/logs/logger.rb

@@ -0,0 +1,33 @@
+# frozen_string_literal: true
+


The Logger SDK will be fully written in a separate PR. The code here represents what's necessary to fulfill the LoggerProvider SDK spec.

kaylareopelle · 2023-10-02T19:00:41Z

logs_sdk/opentelemetry-logs-sdk.gemspec

@@ -24,15 +24,18 @@ Gem::Specification.new do |spec|
  spec.require_paths = ['lib']
  spec.required_ruby_version = '>= 3.0'



Updating dependencies to more recent versions. This came about when I realized the SimpleCov version that was installed didn't include a feature I wanted to use.

kaylareopelle · 2023-10-02T19:06:19Z

logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb

+# Copyright The OpenTelemetry Authors
+#
+# SPDX-License-Identifier: Apache-2.0
+


The specs for SDK LoggerProvider and SDK TracerProvider are extremely similar.

Given this, the code is very similar to the already implemented TracerProvider in the SDK gem.

kaylareopelle · 2023-10-02T23:20:01Z

logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb

+          name ||= ''
+          version ||= ''
+
+          OpenTelemetry.logger.warn(EMPTY_NAME_ERROR) if name.empty?


In the case where an invalid name (null or empty string) is specified, a working Logger MUST be returned as a fallback rather than returning null or throwing an exception, its name SHOULD keep the original invalid value, and a message reporting that the specified value is invalid SHOULD be logged.
source

This logic is drawn from TracerProvider#tracer. The null or empty string will not stop InstrumentationScope from being generated during Logger initialization.

kaylareopelle · 2023-10-02T23:24:56Z

logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb

+          OpenTelemetry.logger.warn(EMPTY_NAME_ERROR) if name.empty?
+
+          @mutex.synchronize do
+            OpenTelemetry::SDK::Logs::Logger.new(name, version, self)


The input provided by the user MUST be used to create an InstrumentationScope instance which is stored on the created Logger.

source

The name and version here are passed to the Logger to create the InstrumentationScope.

kaylareopelle · 2023-10-02T23:26:44Z

logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb

+        #
+        # @param [LogRecordProcessor] log_record_processor The
+        #   {LogRecordProcessor} to add to this LoggerProvider.
+        def add_log_record_processor(log_record_processor)


The LoggerProvider MAY provide methods to update the configuration. If configuration is updated (e.g., adding a LogRecordProcessor), the updated configuration MUST also apply to all already returned Loggers (i.e. it MUST NOT matter whether a Logger was obtained from the LoggerProvider before or after the configuration change). Note: Implementation-wise, this could mean that Logger instances have a reference to their LoggerProvider and access configuration only via this reference.

source

Every logger instance has a reference to the logger provider that created it. (see L50) This gives loggers access to the log record processors.

kaylareopelle · 2023-10-02T23:31:07Z

logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb

+        # @param [optional Numeric] timeout An optional timeout in seconds.
+        # @return [Integer] Export::SUCCESS if no error occurred, Export::FAILURE if
+        #   a non-specific failure occurred, Export::TIMEOUT if a timeout occurred.
+        def shutdown(timeout: nil)


See: https://opentelemetry.io/docs/specs/otel/logs/sdk/#shutdown

Only called once by setting @stopped to true L92

Provides a way to let the caller know whether it succeeded, failed, or timed out by returning OpenTelemetry::SDK::Logs::EXPORT constants (L81, L87, L93)

Implements a timeout through calls to OpenTelemetry::Common::Utilities.timeout_timestamp and OpenTelemetry::Common::Utilities.maybe_timeout (L84, L86)

Invokes shutdown on all the processors (L85 - 89)

kaylareopelle · 2023-10-02T23:59:45Z

logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb

+        # Immediately export all {LogRecord}s that have not yet been exported
+        # for all the registered {LogRecordProcessor}s.
+        #
+        # This method should only be called in cases where it is absolutely
+        # necessary, such as when using some FaaS providers that may suspend
+        # the process after an invocation, but before the {LogRecordProcessor}
+        # exports the completed {LogRecord}s.
+        #
+        # @param [optional Numeric] timeout An optional timeout in seconds.
+        # @return [Integer] Export::SUCCESS if no error occurred, Export::FAILURE if
+        #   a non-specific failure occurred, Export::TIMEOUT if a timeout occurred.
+        def force_flush(timeout: nil)
+          @mutex.synchronize do
+            return Export::SUCCESS if @stopped
+
+            start_time = OpenTelemetry::Common::Utilities.timeout_timestamp
+            results = log_record_processors.map do |processor|
+              remaining_timeout = OpenTelemetry::Common::Utilities.maybe_timeout(timeout, start_time)
+              return Export::TIMEOUT if remaining_timeout&.zero?
+
+              processor.force_flush(timeout: remaining_timeout)
+            end
+
+            results.max || Export::SUCCESS
+          end
+        rescue StandardError => e
+          OpenTelemetry.handle_error(exception: e, message: FORCE_FLUSH_ERROR)
+          Export::FAILURE
+        end


Spec reference: https://opentelemetry.io/docs/specs/otel/logs/sdk/#forceflush

Out of time atm, but I'll circle back with more 1:1 details.

mwear

Looks good. I have a few minor comments.

logs_sdk/lib/opentelemetry/sdk/logs/log_record_processor.rb

mwear · 2023-10-03T00:06:15Z

logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb

+        #   {LogRecordProcessor} to add to this LoggerProvider.
+        def add_log_record_processor(log_record_processor)
+          @mutex.synchronize do
+            @log_record_processors = log_record_processors.dup.push(log_record_processor)


I'm uncertain, but @fbogsany probably knows. I think it has to do with mutating the processors array. Looking at TracerProvider a span gets a reference to processors on start. If a span processor is added after a span is started, and the processors array is mutated, a span will see a processor that wasn't there when it was started. I'm not sure if this would happen in the real world, or if this is what we're protecting against, but that is a reason why we might prefer dup.push over push.

mwear · 2023-10-03T00:16:36Z

logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb

+        rescue StandardError => e
+          OpenTelemetry.handle_error(exception: e, message: FORCE_FLUSH_ERROR)
+          Export::FAILURE
+        end


We might be assuming that processors will handle their own errors, but that might be optimistic. I would be fine with rescuing from StandardError in force_flush and shutdown across the board. I'll let others chime in if they have opinions.

kaylareopelle · 2023-10-04T23:48:50Z

@mwear - Re: this comment

I've updated #shutdown to include error handling to match #force flush in: 3c0dd57

logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb

mwear

LGTM, thanks @kaylareopelle!

* Add OpenTelemetry::SDK::Logs::Export constants * Create OpenTelemetry::SDK::Logs::LoggerProvider * Implement LoggerProvider#logger * Implement LoggerProvider#shutdown * Implement LoggerProvider#force_flush * Create no-op LogRecordProcessor

kaylareopelle · 2023-10-09T22:34:17Z

Thanks, @mwear! I've rebased the branch into one commit.

robbkidd

👍🏻

arielvalentin

Assuming we gain a better understanding of duping the log processors...

arielvalentin · 2023-10-05T00:03:26Z

logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb

+    module Logs
+      # The SDK implementation of OpenTelemetry::Logs::LoggerProvider.
+      class LoggerProvider < OpenTelemetry::Logs::LoggerProvider
+        attr_reader :resource, :log_record_processors


log_record_processors need safe concurrent access. By exposing this as a reader, external callers may gain access to the array in an unsafe way.

This attribute should not be exposed as a public value

It is also not exposed in the spec.

Suggested change

attr_reader :resource, :log_record_processors

attr_reader :resource

fbogsany

No changes yet - I just want to block so I have a chance to read it closely.

logs_sdk/lib/opentelemetry-logs-sdk.rb

fbogsany · 2023-10-17T16:08:15Z

logs_sdk/lib/opentelemetry/sdk/logs/export.rb

+module OpenTelemetry
+  module SDK
+    module Logs
+      # The export module contains result codes for LoggerProvider#force_flush


These should also be result codes for exporters - their use shouldn't just relate to these two methods.

Great point. Updated to be more general in: d63a4e9

fbogsany · 2023-10-17T16:10:42Z

logs_sdk/lib/opentelemetry/sdk/logs/log_record_processor.rb

+        # Called when a {LogRecord} is emitted. Subsequent calls are not
+        # permitted after shutdown is called.
+        # @param [LogRecord] log_record The emitted {LogRecord}
+        # @param [Context] context The resolved Context


What is the "resolved Context" here? Who does the resolution? (Note that I haven't read this part of the spec or all of this PR, or the logs API, so maybe this is answered clearly somewhere else - just asking as a naive reviewer.)

The "resolved Context" here is coming from the Logs SDK spec description for the method arguments.

https://opentelemetry.io/docs/specs/otel/logs/sdk/#onemit

The description has a bit more detail to include that might clear things up. I've updated the description to include the rest of the text in: 5528277

I'm not sure who does the resolution. I'm also not quite sure how we'll use this argument.

I'll consult some of the implementations in other languages and come back with more info.

@fbogsany - I can't find any examples where the context argument is used in other languages.

Python doesn't include it anywhere.

PHP and Java have it as an argument but do not use it in the onEmit functions.

Javascript doesn't include it as an argument in their Simple or Batch processors, but the duck type includes it as an argument.

So I don't have a great answer about who does the resolution. I didn't end up needing this argument in the Simple or Batch processors because I used the SpanContext to pull trace_id, span_id, and trace_flags. Those are the only context-related attributes in the Logs Data Model.

Do you have a sense of what approach you'd prefer Ruby take?

fbogsany · 2023-10-17T16:13:43Z

logs_sdk/lib/opentelemetry/sdk/logs/logger.rb

+    module Logs
+      # The SDK implementation of OpenTelemetry::Logs::Logger
+      class Logger < OpenTelemetry::Logs::Logger
+        attr_reader :instrumentation_scope, :logger_provider


I'm curious why we expose these attributes. In the Tracer and Meter, they're private, so the inconsistency is a little weird.

This attr_reader call has been removed. I'm not sure why I initially broke from Tracer and Meter.

fbogsany · 2023-10-17T16:16:07Z

logs_sdk/lib/opentelemetry/sdk/logs/logger.rb

+        # @api private
+        #
+        # Returns a new {OpenTelemetry::SDK::Logs::Logger} instance. This should
+        # not be called directly. New loggers should be created using
+        # {LoggerProvider#logger}.
+        #
+        # @param [String] name Instrumentation package name
+        # @param [String] version Instrumentation package version
+        # @param [LoggerProvider] logger_provider The {LoggerProvider} that
+        #   initialized the logger
+        #
+        # @return [OpenTelemetry::SDK::Logs::Logger]
+        def initialize(name, version, logger_provider)
+          @instrumentation_scope = InstrumentationScope.new(name, version)
+          @logger_provider = logger_provider
+        end


The @api private comment is our standard way of marking "internal" methods. There is no practical way to enforce it.

fbogsany · 2023-10-17T16:24:14Z

logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb

+        # @return [OpenTelemetry::SDK::Logs::LoggerProvider]
+        def initialize(
+          resource: OpenTelemetry::SDK::Resources::Resource.create,
+          log_record_processors: []


Providing the log_record_processors at creation time is MAY in the spec. We don't do that in the MeterProvider (for metric_readers) or the TracerProvider (for span_processors). Instead, we rely on the add_... methods to add them individually. I would start with not exposing it during creation. We can always add it later if that's a common pattern (and then consider adding to TracerProvider and MeterProvider as well).

I'm happy to remove the arg from the initialize method. I'll double-check about its commonality in other languages. I've also removed the public reader. See: 37befe6

fbogsany · 2023-10-17T16:32:26Z

logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb

+        # @param [optional String] version Instrumentation package version
+        #
+        # @return [OpenTelemetry::SDK::Logs::Logger]
+        def logger(name = nil, version = nil)


This signature is inconsistent between TracerProvider and MeterProvider. In the latter, we used:

def meter(name, version: nil)

so name is required and version is a keyword arg. IIRC we're going to have to add one or two more args at some point 🔜 , so keywords for optional args are vastly preferred.

We should go back and fix this for TracerProvider. I think that can be done in a backwards-compatible fashion.

For MeterProvider, we opted to require name. If a user explicitly passes nil or "", we can handle it, but those are both considered invalid - I'd rather have an explicitly invalid required arg than an invalid default arg.

Sounds great. I prefer the keyword args, I just defaulted to what was in TracerProvider since that was already stable.

Updated in: ca7c15f

logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb

fbogsany · 2023-10-17T16:42:58Z

logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb

+        #   {LogRecordProcessor} to add to this LoggerProvider.
+        def add_log_record_processor(log_record_processor)
+          @mutex.synchronize do
+            @log_record_processors = log_record_processors.dup.push(log_record_processor)


It's worth considering whether it should be valid to add log record processors after @stopped. In the TracerProvider, we warn and don't allow that.

fbogsany · 2023-10-17T16:51:33Z

logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb

+        rescue StandardError => e
+          OpenTelemetry.handle_error(exception: e, message: UNEXPECTED_ERROR_MESSAGE % __method__)
+          Export::FAILURE


In the TracerProvider and MeterProvider we expect the processors/readers to not raise from #shutdown or #force_flush and instead only return one of the 3 supported result codes.

Understood. Fixed in: a619d36

logs_sdk/lib/opentelemetry/sdk/logs/log_record_processor.rb

logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb

Co-authored-by: Francis Bogsanyi <[email protected]>

kaylareopelle · 2024-01-18T18:15:51Z

@fbogsany, checking back in here! Have your concerns been addressed?

kaylareopelle requested review from fbogsany, mwear, robertlaurin, dazuma, ericmustin, arielvalentin, ahayworth, plantfansam and robbkidd as code owners August 25, 2023 19:12

kaylareopelle commented Aug 25, 2023

View reviewed changes

kaylareopelle commented Aug 28, 2023

View reviewed changes

jack-berg reviewed Aug 31, 2023

View reviewed changes

logs_sdk/lib/opentelemetry/sdk/logs/log_record_processor.rb Show resolved Hide resolved

kaylareopelle commented Oct 3, 2023

View reviewed changes

mwear reviewed Oct 3, 2023

View reviewed changes

kaylareopelle force-pushed the sdk-logger-provider branch from 28d4f3a to 3c0dd57 Compare October 4, 2023 23:46

kaylareopelle requested a review from mwear October 4, 2023 23:48

kaylareopelle commented Oct 4, 2023

View reviewed changes

logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb Outdated Show resolved Hide resolved

mwear approved these changes Oct 7, 2023

View reviewed changes

feat: Create Logs SDK LoggerProvider

de7f36c

* Add OpenTelemetry::SDK::Logs::Export constants * Create OpenTelemetry::SDK::Logs::LoggerProvider * Implement LoggerProvider#logger * Implement LoggerProvider#shutdown * Implement LoggerProvider#force_flush * Create no-op LogRecordProcessor

kaylareopelle force-pushed the sdk-logger-provider branch from 7f4b1f1 to de7f36c Compare October 9, 2023 22:33

robbkidd approved these changes Oct 10, 2023

View reviewed changes

arielvalentin approved these changes Oct 10, 2023

View reviewed changes

robertlaurin approved these changes Oct 17, 2023

View reviewed changes

fbogsany requested changes Oct 17, 2023

View reviewed changes

kaylareopelle added 3 commits October 17, 2023 15:44

Update Export module description

d63a4e9

Update #on_emit context arg description

5528277

Remove public reader for log_record_processors

37befe6

kaylareopelle requested a review from fbogsany November 7, 2023 19:41

Merge branch 'main' into sdk-logger-provider

75792a6

kaylareopelle mentioned this pull request Dec 4, 2023

Draft: Logs SDK, OTLP exporter kaylareopelle/opentelemetry-ruby#6

Draft

fbogsany reviewed Dec 5, 2023

View reviewed changes

logs_sdk/lib/opentelemetry/sdk/logs/log_record_processor.rb Outdated Show resolved Hide resolved

fbogsany reviewed Dec 5, 2023

View reviewed changes

logs_sdk/lib/opentelemetry/sdk/logs/log_record_processor.rb Outdated Show resolved Hide resolved

fbogsany reviewed Dec 5, 2023

View reviewed changes

logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb Outdated Show resolved Hide resolved

fbogsany reviewed Dec 5, 2023

View reviewed changes

logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb Outdated Show resolved Hide resolved

fbogsany reviewed Dec 5, 2023

View reviewed changes

logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb Outdated Show resolved Hide resolved

fbogsany reviewed Dec 5, 2023

View reviewed changes

logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb Outdated Show resolved Hide resolved

fbogsany reviewed Dec 5, 2023

View reviewed changes

logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb Outdated Show resolved Hide resolved

fbogsany reviewed Dec 5, 2023

View reviewed changes

logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb Outdated Show resolved Hide resolved

fbogsany reviewed Dec 5, 2023

View reviewed changes

logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb Outdated Show resolved Hide resolved

fbogsany reviewed Dec 5, 2023

View reviewed changes

logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb Outdated Show resolved Hide resolved

Apply suggestions from code review

aba959f

Co-authored-by: Francis Bogsanyi <[email protected]>

kaylareopelle requested a review from fbogsany December 7, 2023 00:50

kaylareopelle and others added 2 commits December 7, 2023 15:00

Merge branch 'main' into sdk-logger-provider

d72c982

Apply suggestions from code review

5f7d674

Co-authored-by: Francis Bogsanyi <[email protected]>

This was referenced Dec 12, 2023

Support YARD's experimental type parser by removing "optional" from type descriptions #1553

Open

Refactor duplication between Traces and Logs #1528

Open

kaylareopelle added 2 commits January 3, 2024 13:34

Merge branch 'main' into sdk-logger-provider

da89235

Merge branch 'main' into sdk-logger-provider

b7e6d8c

fbogsany approved these changes Jan 19, 2024

View reviewed changes

kaylareopelle and others added 2 commits January 19, 2024 17:21

Merge branch 'main' into sdk-logger-provider

6c23382

Merge branch 'main' into sdk-logger-provider

44bba16

fbogsany merged commit cff311c into open-telemetry:main Jan 22, 2024
55 checks passed

github-actions bot mentioned this pull request Jan 22, 2024

release: Release 5 gems #1574

Closed

kaylareopelle mentioned this pull request May 10, 2024

feat: Update Logger#on_emit to create LogRecords #1611

Merged

kaylareopelle deleted the sdk-logger-provider branch July 25, 2024 17:54

kaylareopelle added the logs label Oct 2, 2024

		@@ -24,15 +24,18 @@ Gem::Specification.new do \|spec\|
		spec.require_paths = ['lib']
		spec.required_ruby_version = '>= 3.0'

	attr_reader :resource, :log_record_processors
	attr_reader :resource

feat: Create Logs SDK LoggerProvider #1517

feat: Create Logs SDK LoggerProvider #1517

Conversation

kaylareopelle commented Aug 25, 2023 • edited Loading

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

mwear left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kaylareopelle commented Oct 4, 2023

mwear left a comment

Choose a reason for hiding this comment

kaylareopelle commented Oct 9, 2023

robbkidd left a comment

Choose a reason for hiding this comment

arielvalentin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fbogsany 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

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

kaylareopelle commented Jan 18, 2024

kaylareopelle commented Aug 25, 2023 •

edited

Loading