Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Truncate instead failing if string is too long in writer #304

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

zoltanjuhasz
Copy link

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"))

@Fuuzetsu
Copy link

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

@Xyene
Copy link
Member

Xyene commented Nov 27, 2024

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 1087cbf.

The right place to address this, I think, would be around

| None -> address @ [ "symbol", Interned display_name ]
. 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:

| String of string (** use for strings with a large number of unique values *)

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

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.

3 participants