Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add TypeFactory cache instrumentation #2588

Merged
merged 5 commits into from Apr 6, 2023

Conversation

carterkozak
Copy link
Contributor

==COMMIT_MSG==
Add TypeFactory cache instrumentation
==COMMIT_MSG==

@changelog-app
Copy link

changelog-app bot commented Apr 6, 2023

Generate changelog in changelog/@unreleased

Type

  • Feature
  • Improvement
  • Fix
  • Break
  • Deprecation
  • Manual task
  • Migration

Description

Add TypeFactory cache instrumentation

Check the box to generate changelog(s)

  • Generate changelog entry

type: histogram
tags:
- format
docs: Histogram describing the length of strings parsed from input.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was a rename to jackson-metrics since -registry didn't make a lot of sense in the filename.

long hits = hit.getCount() - hitBefore;
long misses = miss.getCount() - missBefore;

assertThat(hits).isGreaterThan(0);
Copy link
Contributor

Choose a reason for hiding this comment

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

should this be 0 hits?

Suggested change
assertThat(hits).isGreaterThan(0);
assertThat(hits).isZero();

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Internally this ends up around 3 due to various components mapping the same type. It's probably best not to make any asserts at all at this point, then assert the value has increased in the second round

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll update the test to operate directly on the TypeFactory since the ObjectMapper complicates things

missBefore = miss.getCount();
mapper.writeValue(ByteStreams.nullOutputStream(), new SimpleSerializable());
misses = miss.getCount() - missBefore;
assertThat(misses).isZero();
Copy link
Contributor

Choose a reason for hiding this comment

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

should we check that hits > 0 here?

Suggested change
assertThat(misses).isZero();
assertThat(hit.getCount() - hits).isGreaterThan(0);

@bulldozer-bot bulldozer-bot bot merged commit 9751bb3 into develop Apr 6, 2023
@bulldozer-bot bulldozer-bot bot deleted the ckozak/typefactory_instrumentation branch April 6, 2023 21:27
@svc-autorelease
Copy link
Collaborator

Released 7.49.0

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

Successfully merging this pull request may close these issues.

None yet

4 participants