Skip to content

Conversation

@saxenakshitiz
Copy link
Contributor

In case user agent length exceeds max length, the user agent is truncated to the configured max length

@saxenakshitiz saxenakshitiz requested review from a team and avinashkolluru September 30, 2021 17:55
@saxenakshitiz saxenakshitiz changed the title Enforce user agent max length Enforce max length for user agent Sep 30, 2021
@github-actions

This comment has been minimized.

@codecov
Copy link

codecov bot commented Sep 30, 2021

Codecov Report

Merging #261 (9cc2fc8) into main (4470461) will decrease coverage by 0.01%.
The diff coverage is 62.50%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main     #261      +/-   ##
============================================
- Coverage     80.48%   80.46%   -0.02%     
- Complexity     1161     1162       +1     
============================================
  Files           102      102              
  Lines          4494     4500       +6     
  Branches        418      420       +2     
============================================
+ Hits           3617     3621       +4     
- Misses          681      682       +1     
- Partials        196      197       +1     
Flag Coverage Δ
unit 80.46% <62.50%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...er/enrichment/enrichers/UserAgentSpanEnricher.java 67.21% <62.50%> (-0.06%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4470461...9cc2fc8. Read the comment docs.

@jcchavezs
Copy link
Contributor

I am curious about which situation this happens, where did this come from?

private static final String CACHE_CONFIG_MAX_SIZE = "maxSize";
private static final int CACHE_MAX_SIZE_DEFAULT = 10000;
private static final String USER_AGENT_MAX_LENGTH_KEY = "user.agent.max.length";
private static final int DEFAULT_USER_AGENT_MAX_LENGTH = 1000;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious about this number, it looks way too big, could you elaborate on why this number is reasonable?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please look at the test. We are getting user agent string close to 6k in length. That is the reason to bring in this limit. It is still very liberal. However the failure we had observed in prod environment with user agent of length ~6k is not there when user agent is truncated to length of 1000. Actually our code could handle user agent up to length of ~2.5k.

@saxenakshitiz saxenakshitiz merged commit 25b7aaf into main Oct 1, 2021
@saxenakshitiz saxenakshitiz deleted the user_agent_max_length branch October 1, 2021 05:00
@github-actions
Copy link

github-actions bot commented Oct 1, 2021

Unit Test Results

  70 files  ±0    70 suites  ±0   56s ⏱️ +4s
379 tests +1  379 ✔️ +1  0 💤 ±0  0 ❌ ±0 

Results for commit 25b7aaf. ± Comparison against base commit 4470461.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants