From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <d.csapak@proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256)
(No client certificate requested)
by lists.proxmox.com (Postfix) with ESMTPS id 5B874AB1E
for <pmg-devel@lists.proxmox.com>; Wed, 28 Jun 2023 09:27:37 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id 4325F1EBAB
for <pmg-devel@lists.proxmox.com>; Wed, 28 Jun 2023 09:27:37 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com
[94.136.29.106])
(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256)
(No client certificate requested)
by firstgate.proxmox.com (Proxmox) with ESMTPS
for <pmg-devel@lists.proxmox.com>; Wed, 28 Jun 2023 09:27:36 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1])
by proxmox-new.maurer-it.com (Proxmox) with ESMTP id EF48941A50;
Wed, 28 Jun 2023 09:27:35 +0200 (CEST)
Message-ID: <2c99e898-7a3c-2f7e-61df-386dd321a205@proxmox.com>
Date: Wed, 28 Jun 2023 09:27:35 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Content-Language: en-US
To: Stoiko Ivanov <s.ivanov@proxmox.com>, pmg-devel@lists.proxmox.com
References: <20230627194650.34027-1-s.ivanov@proxmox.com>
<20230627194650.34027-2-s.ivanov@proxmox.com>
From: Dominik Csapak <d.csapak@proxmox.com>
In-Reply-To: <20230627194650.34027-2-s.ivanov@proxmox.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
X-SPAM-LEVEL: Spam detection results: 0
AWL 0.016 Adjusted score from AWL reputation of From: address
BAYES_00 -1.9 Bayes spam probability is 0 to 1%
DMARC_MISSING 0.1 Missing DMARC policy
KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record
SPF_PASS -0.001 SPF: sender matches SPF record
T_SCC_BODY_TEXT_LINE -0.01 -
URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See
http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more
information. [main.rs]
Subject: Re: [pmg-devel] [RFC pmg-log-tracker 1/2] rfc3339 timestamps:
counter-offset timezone
X-BeenThere: pmg-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox Mail Gateway development discussion
<pmg-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pmg-devel>,
<mailto:pmg-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pmg-devel/>
List-Post: <mailto:pmg-devel@lists.proxmox.com>
List-Help: <mailto:pmg-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel>,
<mailto:pmg-devel-request@lists.proxmox.com?subject=subscribe>
X-List-Received-Date: Wed, 28 Jun 2023 07:27:37 -0000
High level comment:
isn't the api simply adding the local offset from 'now' ?
so here we add the offset depending on when it was logged,
meaning if the offset changed from the log to now it's still
off to what it was previously?
e.g.
log at 05:00 with +02:00 (epoch at 03:00 UTC)
gets converted to epoch at 05:00 UTC
now it's +01:00 (e.g. after summer/wintertime change)
result from the api is epoch at 04:00 UTC
so i guess we should either add the current timezone offset (since
that's what the api does, or fix it 'correct' by adding the timezone
info directly in the log-tracker for lines where we don't have the
info, and drop it in the api...
otherwise the code looks ok with one nit inline
On 6/27/23 21:46, Stoiko Ivanov wrote:
> the old time-format had no timezone-information so the API treats the
> result as 'timezoned-epoch' and subtracts the timezone-offset
>
> In order to support both formats add the timezone-offset to the
> already offset time - so that the subtraction in the API is canceled
> out.
>
> This can luckily be dropped once we drop support for traditional
> syslog format.
>
> inspired by parse_rfc3339_do in proxmox-time
>
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
> src/main.rs | 27 +++++++++++++++++++++++----
> 1 file changed, 23 insertions(+), 4 deletions(-)
>
> diff --git a/src/main.rs b/src/main.rs
> index e7bffd8..6c4a555 100644
> --- a/src/main.rs
> +++ b/src/main.rs
> @@ -2263,12 +2263,31 @@ fn parse_time_with_year(data: &'_ [u8]) -> Option<(time_t, &'_ [u8])> {
> timestamp_buffer[0..year_time_len].copy_from_slice(year_time);
> timestamp_buffer[year_time_len..timestamp_len].copy_from_slice(timezone);
>
> - match proxmox_time::parse_rfc3339(unsafe {
> + let epoch = match proxmox_time::parse_rfc3339(unsafe {
> std::str::from_utf8_unchecked(×tamp_buffer[0..timestamp_len])
> }) {
> - Ok(ltime) => Some((ltime, data)),
> - Err(_err) => None,
> - }
> + Ok(ltime) => ltime,
> + Err(_err) => return None,
> + };
> +
> + // FIXME: the traditional syslog format does not have timezone information, so the api-backend
> + // shifts the time by the offset (i.e. parse_time_no_year returns timestamps in local time)
> + // readd the TZ-offset here to match the broken format
> + // drop after legacy parsing is dropped.
> + let tz = timezone[0];
> + if tz == b'Z' {
> + return Some((epoch, data));
> + }
> +
> + let hours = (timezone[1] as i32 - 48) * 10 + (timezone[2] as i32 - 48);
> + let mins = (timezone[4] as i32 - 48) * 10 + (timezone[5] as i32 - 48);
nit: i guess it's copied but wouldn't it make more sense to use b'0' instead of 48 here?
at least it would it make more obvious what it does
> + let offset = hours * 3600 + mins * 60;
> + let epoch = match tz {
> + b'+' => epoch + offset as i64,
> + b'-' => epoch - offset as i64,
> + _ => unreachable!(), // already checked in parse_rfc3339
> + };
> + Some((epoch, data))
> }
>
> fn parse_time_no_year(data: &'_ [u8], cur_year: i64, cur_month: i64) -> Option<(time_t, &'_ [u8])> {