Adds tracing id to troubleshoot indexing. #5249
Conversation
app/services/indexer.rb
Outdated
| def self.trace_id_for(druid:, source:) | ||
| SecureRandom.uuid.tap do |trace_id| | ||
| Rails.logger.info("Reindexing #{druid} from #{source} with trace_id=#{trace_id}") | ||
| end | ||
| end |
There was a problem hiding this comment.
Is the long-story-short explanation of all these changes that we want a way to relate log entries and honeybadger exceptions to solr documents? Those are the three spots where these IDs wind up, if I'm reading this right.
There was a problem hiding this comment.
That's correct. We have Solr docs that are out of date; we don't know how they got written. Is there a race condition? Is stale data being indexed? Are Solr commits being dropped? Something else?
app/services/indexer.rb
Outdated
| def self.reindex(cocina_object:, trace_id:) | ||
| return unless Settings.solr.enabled | ||
|
|
||
| solr_doc = Indexing::Builders::DocumentBuilder.for( | ||
| model: cocina_object | ||
| model: cocina_object, | ||
| trace_id: | ||
| ).to_solr |
There was a problem hiding this comment.
I'm wondering if we need to add this arg to Indexer.reindex and Indexer.reindex_later since both of these methods already have:
- A Cocina object, which should always have the druid in question (?), and
Kernel#caller_locations, which should get us the path back to the caller
If we could make ☝🏻 work, that would keep trace_id from leaking all over the place.
There was a problem hiding this comment.
I think it is likely that we're going to want to push trace_id out to other systems and include it in the messages / calls to DSA that request indexing. For this, we'll want trace_id leaking all over the place.
There was a problem hiding this comment.
OK, then would ☝🏻 work for DSA-internal use? If so, then we could cut down the changes in this PR and incorporate the remote trace stuff when we work on that.
There was a problem hiding this comment.
@mjgiarlo I've reduced the footprint using your suggested technique. I still left the method signature hooks in place so that an existing trace_id can be passed in.
justinlittman
force-pushed
the
index_tracing
branch
3 times, most recently
from
9e1f4a3 to
76d4030
Compare
justinlittman
force-pushed
the
index_tracing
branch
from
76d4030 to
9c7c870
Compare
refs #5231
Why was this change made? 🤔
To allow better debugging of indexing.
How was this change tested? 🤨
Stage, unit
⚡ ⚠ If this change has cross service impact, including data writes to shared file systems, run integration tests and/or test in [stage|qa] environment, in addition to specs. ⚡