tanmayrauth commented on PR #1655:
URL: https://github.com/apache/iceberg-go/pull/1655#issuecomment-5227304777
Thanks @laskoviymishka for the review.
I went with your lean and now honor a server-side disable. Enablement
requires both a client opt-in and that the merged config doesn't resolve the
key to false, so a server override setting rest-metrics-reporting-enabled=false
suppresses a client that opted in — same way a Java client resolves it, so an
operator can switch reporting off fleet-wide. A server default still can't flip
off a client (client props win over defaults in the merge), and the direction
that actually matters is unchanged: a server can never turn reporting on.
For the parity divergence, I added a Metrics reporting section to
website/src/configuration.md — there's no CHANGES.md in the repo. It documents
the three properties, calls out that we default off where Java defaults on
(METRICS_REPORTING_ENABLED_DEFAULT=true), and notes that
rest-metrics-reporting-timeout-ms is a Go-only extension with no
Java/PyIceberg/Rust equivalent.
On the smaller items: added the positive test case that advertises the
metrics endpoint explicitly, so the resolveEndpoints/endpointFromString
template parse gets exercised instead of just the fallback path — plus
server-override-false and server-default-false cases for the new behavior.
close() now uses time.NewTimer with a deferred Stop instead of time.After, with
a note that the timeout arm can return before the workers have fully stopped
so nobody tears down a shared transport assuming otherwise. Registered
t.Cleanup(d.close) on the two directly-constructed dispatcher tests, added the
one-liner on the dispatcher's ctx/cancel fields explaining the deliberate
lifecycle-context exception, and put debug logging on the two swallowed
splitIdentForPath/reqPath branches so a regression surfaces instead of metrics
quietly disappearing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
