Skip to content

Conversation

@swistak35
Copy link
Contributor

No description provided.

@swistak35
Copy link
Contributor Author

#<RailsEventStore::Client:0x2acc394294bc>
  - repository: #<RubyEventStore::InstrumentedRepository:0x2acc39436f2c inner_repository=#<RailsEventStoreActiveRecord::EventRepository:0x2acc39429494>>
  - dispatcher: #<RubyEventStore::InstrumentedDispatcher:0x2acc39436edc dispatcher=#<RubyEventStore::ComposedDispatcher:0x2acc39436fa4 dispatchers=[#<RubyEventStore::ImmediateAsyncDispatcher:0x2acc39436ff4 scheduler=#<RailsEventStore::ActiveJobScheduler:0x2acc3943701c>>, #<RubyEventStore::PubSub::Dispatcher:0x2acc39436fcc>]>>
  - mapper: #<RubyEventStore::Mappers::InstrumentedMapper:0x000055987286dde0 @mapper=#<RubyEventStore::Mappers::Default:0x2acc394374a4 transformations=[#<RubyEventStore::Mappers::Transformation::DomainEvent:0x2acc394373b4>, #<RubyEventStore::Mappers::Transformation::EventClassRemapper:0x2acc3943747c>, #<RubyEventStore::Mappers::Transformation::SymbolizeMetadataKeys:0x2acc39437468>, #<RubyEventStore::Mappers::Transformation::Serialization:0x2acc39437440 serializer=Psych>, #<RubyEventStore::Mappers::Transformation::SerializedRecord:0x2acc3943738c>]>, @instrumentation=ActiveSupport::Notifications>

@swistak35
Copy link
Contributor Author

What would you say about something like this?

#<RailsEventStore::Client:0x2acc394294bc
  repository=#<RubyEventStore::InstrumentedRepository:0x2acc39436f2c
    inner_repository=#<RailsEventStoreActiveRecord::EventRepository:0x2acc39429494>>
  dispatcher=#<RubyEventStore::InstrumentedDispatcher:0x2acc39436edc
    dispatcher=#<RubyEventStore::ComposedDispatcher:0x2acc39436fa4
      dispatchers=[
        #<RubyEventStore::ImmediateAsyncDispatcher:0x2acc39436ff4
          scheduler=#<RailsEventStore::ActiveJobScheduler:0x2acc3943701c>>,
        #<RubyEventStore::PubSub::Dispatcher:0x2acc39436fcc>
      ]>>
  mapper=#<RubyEventStore::Mappers::InstrumentedMapper:0x000055987286dde0
    mapper=#<RubyEventStore::Mappers::Default:0x2acc394374a4
      transformations=[
        #<RubyEventStore::Mappers::Transformation::DomainEvent:0x2acc394373b4>,
        #<RubyEventStore::Mappers::Transformation::EventClassRemapper:0x2acc3943747c>,
        #<RubyEventStore::Mappers::Transformation::SymbolizeMetadataKeys:0x2acc39437468>,
        #<RubyEventStore::Mappers::Transformation::Serialization:0x2acc39437440 serializer=Psych>,
        #<RubyEventStore::Mappers::Transformation::SerializedRecord:0x2acc3943738c>
      ]>,
    instrumentation=ActiveSupport::Notifications>>

@mpraglowski
Copy link
Member

I would prefer something like:

#<RailsEventStore::Client:0x2acc394294bc
  repository=#<RailsEventStoreActiveRecord::EventRepository:0x2acc39429494, instrumented by: #<RubyEventStore::InstrumentedRepository:0x2acc39436f2c>>
  dispatcher=#<RubyEventStore::ComposedDispatcher:0x2acc39436fa4
      dispatchers=[
        #<RubyEventStore::ImmediateAsyncDispatcher:0x2acc39436ff4
          scheduler=#<RailsEventStore::ActiveJobScheduler:0x2acc3943701c>>,
        #<RubyEventStore::PubSub::Dispatcher:0x2acc39436fcc>
      ], instrumented by: #<RubyEventStore::InstrumentedDispatcher:0x2acc39436edc>>
  mapper=#<RubyEventStore::Mappers::Default:0x2acc394374a4
      transformations=[
        #<RubyEventStore::Mappers::Transformation::DomainEvent:0x2acc394373b4>,
        #<RubyEventStore::Mappers::Transformation::EventClassRemapper:0x2acc3943747c>,
        #<RubyEventStore::Mappers::Transformation::SymbolizeMetadataKeys:0x2acc39437468>,
        #<RubyEventStore::Mappers::Transformation::Serialization:0x2acc39437440 serializer=Psych>,
        #<RubyEventStore::Mappers::Transformation::SerializedRecord:0x2acc3943738c>
      ], instrumented by: #<RubyEventStore::Mappers::InstrumentedMapper:0x000055987286dde0>>,
    instrumentation=ActiveSupport::Notifications>

When not instrumented components are used just the instrumented by section will be missing, other will stay the same.

@mostlyobvious
Copy link
Member

mostlyobvious commented Aug 13, 2019

instrumented by: #RubyEventStore::InstrumentedDispatcher:0x2acc39436edc

Would it be clearer to show actual instrumenter here like AS::Notifications, not the wrapper around it?

@mpraglowski
Copy link
Member

We should not care about the object_id of internal components, so IMHO yes, instrumented by: ActiveSupport::Notifications should be enough here.

@swistak35
Copy link
Contributor Author

Just to recap:

  1. in the beginning we had big blob of text, which had all information, but was only useful when using some find tools. Also it was just annoying to retrieve a variable with event store in console, as it immediately printed that wall of text.
  2. now we have just vague #<RailsEventStore::Client:0x2b253480a5b4>, which is nice that it doesn't print a wall of text, but it doesn't tell about the internals [1975a18]

I'm ok with skipping object ids. I would not however not do things like #<RailsEventStoreActiveRecord::EventRepository:0x2acc39429494, instrumented by: #<RubyEventStore::InstrumentedRepository:0x2acc39436f2c>> unless it's really needed for readability. inspect is used primarily to inspect an object and it's internals, so leaking information about internal components (and object tree) is IMHO not only acceptable, but even expected behaviour. I imagine scenario where someone is debugging why something is wrong withing event store -- maybe because we have bug, or maybe because the user used event store in a way we haven't predicted to optimize and something is not behaving as he expects, and in these cases, it's crucial to be able to "just inspect" the object. I think that output would make it even harder to debug issues like #561 .

IMHO:

  1. 👎 for #<RailsEventStoreActiveRecord::EventRepository:0x2acc39429494, instrumented by: #<RubyEventStore::InstrumentedRepository:0x2acc39436f2c>> -- because it's simple and pretty obvious object composition and I don't think there's any gain from such formatting.
  2. 👍 for skipping details sometimes, i.e. I don't imagine printing subscriptions in useful way without some really custom formatting

@mpraglowski
Copy link
Member

I would say both 👍 for 1st, and also for 2nd (skipping details) as inspect on subscriptions could be again the wall of text. BTW the subscriptions part I would like to be able to see in RES browser.

@mostlyobvious
Copy link
Member

A bit stale already and no momentum to push this forward. Closing, but not discouraging.

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.

4 participants