Modify context path of OTEL transformers #4134
Conversation
As of 0.119.0, the parser collector modifies certain paths of transforms. While this is not an error, the resulting log can look a little alarming, so we rewrote some of our transforms to match. Issues: [PGO-2268]
|
|
||
| // https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/-/pkg/ottl/ottlfuncs#parsejson | ||
| `set(cache, ParseJSON(body["original"]))`, | ||
| `set(log.cache, ParseJSON(log.body["original"]))`, |
There was a problem hiding this comment.
🤔 Do we still have to set context=log? If so, what a strange change they made.
There was a problem hiding this comment.
We do not. I'm not sure of the benefit of this change.
That said, is it expected for almost every emission to be LogRecord #0?
There was a problem hiding this comment.
That sounds like an index, maybe? The batch time is quite small by default, so one-at-a-time sounds plausible.
cbandy
left a comment
cbandy
left a comment
There was a problem hiding this comment.
I couldn't pay attention the whole way through. LGTM
| # https://git.postgresql.org/gitweb/?p=postgresql.git;hb=REL_10_0;f=src/backend/utils/error/elog.c#l2246 | ||
| # https://git.postgresql.org/gitweb/?p=postgresql.git;hb=REL_17_0;f=src/backend/utils/error/elog.c#l2671 | ||
| - set(time, Time(cache["timestamp"], "%F %T.%L %Z")) | ||
| - set(log.time, Time(log.cache["timestamp"], "%F %T.%L %Z")) where IsString(log.cache["timestamp"]) |
There was a problem hiding this comment.
👍🏻 I don't mind being safe here. Did something warn? I expect Postgres always includes a timestamp.
There was a problem hiding this comment.
I can't remember where (pgbouncer, patroni?) but at some point I caused a log to come through that didn't have a timestamp. That might've been when I was trying to reproduce the bug Drew was checking out? But I thought we might want to be safe wherever we're doing a time transform just in case.
* Modify context path of OTEL transformers As of 0.119.0, the parser collector modifies certain paths of transforms. While this is not an error, the resulting log can look a little alarming, so we rewrote some of our transforms to match. * Handle pgBouncer 1.24.0 During testing, some pgbouncer metrics were seen to cause errors when accessing PG through pgbouncer; in addition, we found that 1.24.0 introduced a change to the `show databases` columns. This PR addresses those errors and that change. NOTE: Even when not using NULL values, sqlqueryreceiver will still warn when it finds a NULL value in some row that it scans. Issue: PGO-2268 See: 7a00601
Checklist:
Type of Changes:
What is the current behavior (link to any open issues here)?
As of 0.119.0, the parser collector modifies certain paths of transforms. While this is not an error, the resulting log can look a little alarming, so we rewrote some of our transforms to match.
What is the new behavior (if this is a feature change)?
We handle the transforms.
Other Information:
Issues: [PGO-2268]