magic-trace icon indicating copy to clipboard operation
magic-trace copied to clipboard

Truncate instead failing if string is too long in writer

Open zoltanjuhasz opened this issue 1 year ago • 2 comments

When a symbol name that is longer than 32k is encountered magic-trace stops processing the collected data. This PR truncates the long string instead if failing.

Original error:

[ Decoding, this takes a while... ]
(monitor.ml.Error
 (Failure "string too long for FTF trace: 60872 is over the limit of 32kb")
 ("Raised at Stdlib.failwith in file \"stdlib.ml\", line 29, characters 17-33"
  "Called from Tracing_zero__Writer.set_string_slot in file \"vendor/tracing/zero/writer.ml\", line 158, characters 7-89"
  "Called from Tracing_zero__Writer.intern_string in file \"vendor/tracing/zero/writer.ml\", line 182, characters 2-32"
  "Called from Base__Hashtbl.find_or_add.(fun) in file \"src/hashtbl.ml\", line 427, characters 20-30"
  "Called from Tracing__Trace.Baked_args.bake in file \"vendor/tracing/src/trace.ml\", line 96, characters 27-57"
  "Called from Tracing__Trace.Baked_args.create.(fun) in file \"vendor/tracing/src/trace.ml\", line 110, characters 39-61"
  "Called from Base__List.count_map in file \"src/list.ml\", line 483, characters 13-17"
  "Called from Tracing__Trace.writer_adapter in file \"vendor/tracing/src/trace.ml\", line 171, characters 19-43"
  "Called from Base__List0.iter in file \"src/list0.ml\", line 60, characters 4-7"
  "Called from Magic_trace_lib__Trace_writer.flush in file \"src/trace_writer.ml\", line 447, characters 2-404"
  "Called from Magic_trace_lib__Trace_writer.add_event in file \"src/trace_writer.ml\", line 463, characters 54-82"
  "Called from Magic_trace_lib__Trace_writer.ret in file \"src/trace_writer.ml\", line 680, characters 2-55"
  "Called from Magic_trace_lib__Trace_writer.check_current_symbol in file \"src/trace_writer.ml\", line 696, characters 4-27"
  "Called from Magic_trace_lib__Trace_writer.write_event in file \"src/trace_writer.ml\", line 1170, characters 10-83"
  "Called from Async_kernel__Pipe.iter_without_pushback.(fun).loop in file \"src/pipe.ml\", line 917, characters 10-13"
  "Called from Async_kernel__Job_queue.run_jobs in file \"src/job_queue.ml\", line 180, characters 6-47"
  "Caught by monitor Monitor.protect"))

zoltanjuhasz avatar Oct 01 '24 02:10 zoltanjuhasz

I have tried this patch on top of latest version and it makes magic-trace a lot more usable :+1:

Fuuzetsu avatar Nov 19 '24 05:11 Fuuzetsu

Thanks for the patch!

I think we shouldn't silently truncate these strings, and leave it up to the caller to truncate (or perhaps reformat) the symbols as appropriate. For reference, see how a different variant of this issue was patched in https://github.com/janestreet/magic-trace/commit/1087cbf4888787a6704200e46dffaa50220bfadc.

The right place to address this, I think, would be around https://github.com/janestreet/magic-trace/blob/d36c159dc7d68ce84f4ef5a6538138961ef723f2/src/trace_writer.ml#L385. See the big comment above that logic.

In fact, I think you could try using String instead of Interned here, if the string is too large to be interned: https://github.com/janestreet/magic-trace/blob/d36c159dc7d68ce84f4ef5a6538138961ef723f2/vendor/tracing/src/trace_intf.ml#L6

It might work! But otherwise, I think we should truncate display_name at the callsite, and suffix it with something like [truncated].

Xyene avatar Nov 27 '24 04:11 Xyene