brimdata / brimcap

Convert pcap files into richly-typed ZNG summary logs (Zeek, Suricata, and more)
BSD 3-Clause "New" or "Revised" License
78 stars 8 forks source link

ts field in localtime or set timezone #352

Closed nnmmaaoo closed 3 months ago

nnmmaaoo commented 3 months ago

Hey, i am working on figuring out a way to display brimcap analyze results ts field in a different timezone from UTC (atleast in .Local()). I am struggling to find, which part of the codebase is responsible for translation of for example: zeeks ts that is supplied in unixtime/zeek floa time format. Could you, please, point me in the right direction? Or should i be looking inside zed library?

philrz commented 3 months ago

@nnmmaaoo: Unfortunately there's not a simple way to achieve this with the tooling as it stands currently, though we recognize it's a reasonable thing to want to do and have been discussing the best way to go about addressing it. I've got a couple questions just to clarify which short/long-term approaches would address your use case:

  1. Is your use of Brimcap in support of data that's ultimately viewed in Zui? I ask because, as this is effectively a "data presentation" issue, one way we might go about addressing it is at the Zui layer, but that would not suffice if you were taking the data output from brimcap analyze for use in other contexts.

  2. Do you have one particular timezone that you're trying to render the time values in, or do you need a flexible approach that can easily adapt to many different local times depending on where a user is running? Obviously a comprehensive approach like the latter is ideal, but if you were just trying to solve the problem for one particular time zone in the short term we might be able to put together a quick "hacky" approach that might get you up & running even sooner.

nnmmaaoo commented 3 months ago

Initial problem that i was trying to fix is this: zui displays 2 different dates after analyzing a pcap file, if the timezone setting in zui is not UTC. One is forced UTC in the results pane and one in the details pane (the one that appears on the right) is translated into the zui tz. Following screencap has a UTC+3 timezone set in zui settings. image

Is your use of Brimcap in support of data that's ultimately viewed in Zui?

Yes. For now i have achieved this with a .map to transform the data inside zui. In apps/zui/src/views/results-pane/context.tsx i added the following to the function useContextValue

+  const zone = useTimeZone()
+  results.data.map(function(e) {
+    if (!(e.fields[1].value.type instanceof TypeOfTime)) {
+      return
+    }
+    e.fields[1].value.value = moment(new Date(e.fields[1].value.value))
+      .tz(zone)
+      .toISOString(true)
+  })

Its dirty, since it digs insde zed.Value ts type, but it works. Also it utilizes zui timezone setting. But i dont think that this is a great idea, since this code is gonna be executed on every single load of the results table. But if there is no good way to make brimcap aware of a timezone - this will do.

Ideally i wanted to add either a flag or a respected env to the functionality of brimcap analyze to convert ts before its piped to zed by zui. And for zui to view already "fixed" data.

philrz commented 3 months ago

@nnmmaaoo: Thanks for providing those clarifications. I've had to take some time to reassess the improvements we might make at the different layers here, so now I can better summarize.

Indeed, the inconsistent handling of Zui's Timezone setting has been a known problem for some time and the history is tracked in https://github.com/brimdata/zui/issues/1057, including what you observed where the results panel didn't reflect the timezone while the Detail panel did. As the comments there describe, over time this problem has actually gotten worse, as in the last GA Zui release v1.17.0, the same code that renders the results panel is now used in the Detail panel as well, so at this point the Timezone setting has embarrassingly lost all meaning. I suppose you've maybe not noticed yet because the modified code you showed was probably put in place on an older Zui code version.

The only reason we've not gotten to fixing this problem sooner is because there's been other priorities, but since multiple users including yourself have bumped into this over the years, it definitely feels like it's time we fix this in Zui, so I'll be working with our Dev team to address it ASAP. The fact you were able to come up with a workaround (even if it's maybe "dirty") indicates it shouldn't be too difficult.

Since at first I wasn't sure if your problem was specific to Zui, I did look into what improvements we might make at the Zed layer as well. https://github.com/brimdata/zed/issues/5221 goes into some detail on research and proposals, so you're welcomed to look that over and chime in with a comment if you have any reaction. In some ways that overlaps with your closing remark:

Ideally i wanted to add either a flag or a respected env to the functionality of brimcap analyze to convert ts before its piped to zed by zui. And for zui to view already "fixed" data.

Right now the Zed data model is such that time-typed values (such as ts in the Brimcap use case) are a "signed 64-bit integer as nanoseconds from epoch". So as long as that remains true (as I expect it will), I see limited options to "fix" ts as you indicated before the data lands in Zui. For instance, if the strftime enhancement described in https://github.com/brimdata/zed/issues/5221 was implemented, it would become possible to add shaping Zed code in a Brimcap config to populate an additional string-based field that would present what's in the ts value in an alternate time zone, and that string-based field would flow through to the Zed pool and shown in Zui, and if users knew to look at that particular field for the "fixed" time presentation, then I guess it solves the problem. But the controls in the app like the stacked bar chart and time pins are still reliant on having a "real" time value and not just a string presentation of something that looks like a timestamp, so it doesn't seem like it would be feasible to replace the ts field unless you were willing to lose a bunch of app functionality. In this regard it feels cleaner to have the Zed layer continue to store time values as it does today and let Zui do the conversion for final presentation using the Timezone setting (once we fix that 😛). Among other things, this has the benefit that users in different timezones could both be looking at the same pool contents and have wholly separate timezone presentation in their respective Zui apps. I'm not sure if that matches your use case though.

In closing, I'd recommend you "subscribe" to https://github.com/brimdata/zui/issues/1057 and https://github.com/brimdata/zed/issues/5221 if you're interested in tracking ongoing progress in this area. Since I don't think we're likely to change anything specifically within Brimcap related to this, I'm inclined to close this particular issue, but I'll hold it open for a bit just in case you have responses to what I've just written here. Thanks for your interest in the software!

nnmmaaoo commented 3 months ago

Ok, i understand. Then my solution will have to suffice. Thank you for your explanation.

philrz commented 3 months ago

Thanks @nnmmaaoo. We actually have a new Zui PR https://github.com/brimdata/zui/pull/3139 already in progress with some of this timezone support working, so if that all gets wrapped up and merged, we've love it if you could give it a try in a Zui Insiders release and see if it's a satisfactory alternative to what you coded up yourself. I'll ping you here when it's ready.

philrz commented 2 months ago

@nnmmaaoo: Just wanted to let you know that we did merge some good improvements to the Time Zone support in Zui. I'm, not sure if you're familiar with Zui Insiders, but it's our "daily build" approach for making beta-style features available early for preview/test before our next GA Zui release. It's built as a separate app and starts its own Zed lake storage at a separate filesystem location and on a separate network port so you can have it running side-by-side with "regular" Zui without any crossover between the two in terms of data storage.

If you're game to give it a try, we'd love to hear your feedback. The latest Zui Insiders release 1.17.1-insiders.14 includes the changes. We also have a brief article at https://zui.brimdata.io/docs/features/Time-Display that explains a bit more about how to use it. Hope you're up to give it a spin, and please do let us know how it goes!

nnmmaaoo commented 2 months ago

Thank you. Ill try the insider build when i get the chance