fix(llmobs): prevent dots in evaluation metric labels #15297
Conversation
|
|
Bootstrap import analysisComparison of import times between this PR and base. SummaryThe average import time from this PR is: 250 ± 3 ms. The average import time from base is: 263 ± 5 ms. The import time difference between this PR and base is: -12.9 ± 0.2 ms. Import time breakdownThe following import paths have shrunk:
|
Performance SLOsComparing candidate anis.amar/MLOB-4557 (238359a) with baseline main (2ccf506) 📈 Performance Regressions (1 suite)📈 iastaspectsospath - 24/24✅ ospathbasename_aspectTime: ✅ 4.261µs (SLO: <10.000µs 📉 -57.4%) vs baseline: -1.1% Memory: ✅ 39.931MB (SLO: <41.000MB -2.6%) vs baseline: +5.0% ✅ ospathbasename_noaspectTime: ✅ 1.083µs (SLO: <10.000µs 📉 -89.2%) vs baseline: -0.3% Memory: ✅ 39.951MB (SLO: <41.000MB -2.6%) vs baseline: +5.2% ✅ ospathjoin_aspectTime: ✅ 6.237µs (SLO: <10.000µs 📉 -37.6%) vs baseline: +0.8% Memory: ✅ 39.774MB (SLO: <41.000MB -3.0%) vs baseline: +4.6% ✅ ospathjoin_noaspectTime: ✅ 2.323µs (SLO: <10.000µs 📉 -76.8%) vs baseline: +1.2% Memory: ✅ 39.852MB (SLO: <41.000MB -2.8%) vs baseline: +4.8% ✅ ospathnormcase_aspectTime: ✅ 3.558µs (SLO: <10.000µs 📉 -64.4%) vs baseline: -0.3% Memory: ✅ 39.813MB (SLO: <41.000MB -2.9%) vs baseline: +4.5% ✅ ospathnormcase_noaspectTime: ✅ 0.576µs (SLO: <10.000µs 📉 -94.2%) vs baseline: +1.6% Memory: ✅ 39.813MB (SLO: <41.000MB -2.9%) vs baseline: +5.1% ✅ ospathsplit_aspectTime: ✅ 4.849µs (SLO: <10.000µs 📉 -51.5%) vs baseline: +0.3% Memory: ✅ 39.774MB (SLO: <41.000MB -3.0%) vs baseline: +4.7% ✅ ospathsplit_noaspectTime: ✅ 1.605µs (SLO: <10.000µs 📉 -83.9%) vs baseline: ~same Memory: ✅ 39.793MB (SLO: <41.000MB -2.9%) vs baseline: +4.7% ✅ ospathsplitdrive_aspectTime: ✅ 4.147µs (SLO: <10.000µs 📉 -58.5%) vs baseline: 📈 +12.1% Memory: ✅ 39.754MB (SLO: <41.000MB -3.0%) vs baseline: +4.7% ✅ ospathsplitdrive_noaspectTime: ✅ 0.703µs (SLO: <10.000µs 📉 -93.0%) vs baseline: +0.3% Memory: ✅ 39.852MB (SLO: <41.000MB -2.8%) vs baseline: +4.8% ✅ ospathsplitext_aspectTime: ✅ 4.758µs (SLO: <10.000µs 📉 -52.4%) vs baseline: +2.6% Memory: ✅ 39.931MB (SLO: <41.000MB -2.6%) vs baseline: +5.0% ✅ ospathsplitext_noaspectTime: ✅ 1.388µs (SLO: <10.000µs 📉 -86.1%) vs baseline: ~same Memory: ✅ 39.951MB (SLO: <41.000MB -2.6%) vs baseline: +5.1% 🟡 Near SLO Breach (1 suite)🟡 packagespackageforrootmodulemapping - 4/4✅ cache_offTime: ✅ 339.768ms (SLO: <354.300ms -4.1%) vs baseline: -0.6% Memory: ✅ 40.481MB (SLO: <41.500MB -2.5%) vs baseline: +6.1% ✅ cache_onTime: ✅ 0.383µs (SLO: <10.000µs 📉 -96.2%) vs baseline: -0.1% Memory: ✅ 40.308MB (SLO: <41.000MB 🟡 -1.7%) vs baseline: +5.9%
|
Kyle-Verhoog
left a comment
Kyle-Verhoog
left a comment
There was a problem hiding this comment.
👍 lgtm. We should add a release note (docs) about the change - i think fix is fine for this change.
Not required but it would be much appreciated if you could add a shared test here: https://github.com/DataDog/llm-obs/blob/234c6b76807b426416614aa60c1ec51f2ca61a02/test/test_sdk.py#L877 so that we ensure other SDKs will implement this validation logic too 🙏
EVP interprets dots as nested objects (e.g., "field.1" becomes
{field: {1: value}}), causing confusion for customers submitting
custom evaluations with dots in label names.
Add validation to reject labels containing dots and raise a clear
error message directing users to use alternative naming conventions.
aniszoubiramar
force-pushed
the
anis.amar/MLOB-4557
branch
from
4c4a0eb to
238359a
Compare
FouadWahabi
removed request for
a team,
KowalskiThomas,
P403n1x87,
brettlangdon and
quinna-h
Description
EVP interprets dots as nested objects (e.g., "field.1" becomes {field: {1: value}}), causing confusion for customers submitting custom evaluations with dots in label names.
Add validation to reject labels containing dots and raise a clear error message directing users to use alternative naming conventions.
Testing
Added a unit test to verify the expected behavior.
Risks
None
Additional Notes
JIRA ticket: https://datadoghq.atlassian.net/browse/MLOB-4557