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.

Sign up for GitHub

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

Jump to bottom

Config ErrorHandlingDeserializer to DLQ unparsable messages #134

Merged
merged 1 commit into from
Oct 31, 2023
Merged

Config ErrorHandlingDeserializer to DLQ unparsable messages #134

merged 1 commit into from
Oct 31, 2023

Conversation

smozely
Copy link
Contributor

@smozely smozely commented Oct 31, 2023

Main thing here is configuring org.springframework.kafka.support.serializer.ErrorHandlingDeserializer which delegates to a real Deserializer and if that fails it does some magic to the message via Headers so the spring consumer code knows to reject it.

Notes

There then was a bit of faff because when a message is rejected in this way it has a "value" that is a Byte Array. So the producer a ByteArraySerializer to write it.

But when a message is rejected because the consumer code failed, it still needs to use the JsonSerializer to write it

And there might be a way to do it, but from a bit of time spent didn't quite get it ... and the JsonSerializer already hadnles being passed a ByteArray, it will JSON encode that byte array as a JSON string (see the example in the test)

... The end result being if this occurs the message that ends up in the DLQ isn't super useful to us, but it should have had an error logged and it is for the corner case of getting invalid data into the queue again.

@smozely smozely requested a review from vimto October 31, 2023 04:11
vimto
vimto approved these changes Oct 31, 2023
View reviewed changes
Copy link
Contributor

@vimto vimto left a comment

Choose a reason for hiding this comment

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

Looks good. Are we using the same strategy in other consumers, e.g. in the Hilltop crawler?

@smozely
Copy link
Contributor Author

smozely commented Oct 31, 2023

Looks good. Are we using the same strategy in other consumers, e.g. in the Hilltop crawler?

Writing here for posterity, yes the behavior for Kafka streams seems to be fail-fast so anything unexpected will cause the streams to just quit. Which is a problem in a different way, that processing might stop and go unnoticed

@smozely smozely merged commit 102cf9d into main Oct 31, 2023
1 check passed
@smozely smozely deleted the deseralization-error-handler branch October 31, 2023 22:02
smozely added a commit that referenced this pull request Dec 6, 2023
@smozely
Config ErrorHandlingDeserializer to DLQ unparsable messages (#134)
94c39c9
smozely added a commit that referenced this pull request Dec 6, 2023
@smozely
Config ErrorHandlingDeserializer to DLQ unparsable messages (#134)
8404475
smozely added a commit that referenced this pull request Dec 7, 2023
@smozely
Config ErrorHandlingDeserializer to DLQ unparsable messages (#134)
6b16d0f
smozely added a commit that referenced this pull request Dec 7, 2023
@smozely
Config ErrorHandlingDeserializer to DLQ unparsable messages (#134)
04071f4
smozely added a commit that referenced this pull request Dec 7, 2023
@smozely
Config ErrorHandlingDeserializer to DLQ unparsable messages (#134)
450be76
smozely added a commit that referenced this pull request Dec 7, 2023
@smozely
Config ErrorHandlingDeserializer to DLQ unparsable messages (#134)
805000f
smozely added a commit that referenced this pull request Mar 25, 2024
@smozely @vimto
@MartyP233 @dependabot @swapnasam @wtcarter
Water Use (#195)
9b8a459 
* Update IngestAPI with changes to accommodate usage (#91)

* Improve logging configuration

* Update IngestAPI to accept and store allocation messages with usage

* Message format tweaks

* Make Metered Allocations nullable in API

* Add sourceId to ingested water allocation

* Update water allocation consumer (#92)

* Improve logging configuration

* Update IngestAPI to accept and store allocation messages with usage

* Message format tweaks

* Make Metered Allocations nullable in API

* Add sourceId to ingested water allocation

* Remove unused code

* Updated WaterAllocationMessage and Consumer to handle allocations by consent (rather than area)

* Fix typo

* Ensure Cache manifest for new PlanLimit queries is periodically updated

* Fix failing tests by truncating datetime before comparison to avoid precision differences.

* Update GeoJsonControllerIntegrationTest to reflect new plan limits controller, and remove redundant test

* Changes from PR review

* Ensure only Rainfall measurements are used in the Rainfall queries (#96)

* Manager - Kafka Consumer for Observations (#97)

* Manager - Hilltop Crawler Changes

* Shuffle files around

* martin/aggregate water use (#95)

* add water use daily aggregation sql view

* optimise sql query performance

* Adding Error handling to Observations Consumer

---------

Co-authored-by: Martin Peak <[email protected]>

* Hilltop Crawler (#98)

* Manager - Hilltop Crawler Changes

* Shuffle files around

* Crawler - Updates

* Intergration testing

* Improve DB naming

* Big refactor of test setup

* Correct time to wait between quiet polls

* Add global error handling around task

* Add check to make sure Virtual Measurement matches Measurement Name

* Add a bunch to the readme

* Add views to aggregate daily allocations and usage by area (#100)

* Add views to aggregate consent allocations and usage by day

* Start writing tests

* Allow easier setup and assertion against test data, start adding more tests

* Add tests for various allocation scenarios

* Add failing test for aggregating data across different areas

* Simplify views

* Splitting out the calculation of daily useage by area

* Add more assertions for aggregation of different areas

* water_allocation.meter should correlate with observation_site.name

* Format code

* Feedback from PR review

---------

Co-authored-by: Steve Mosley <[email protected]>

* Remove unnesscary `yield`

* Adding index on next_fetch_at column

* Add some more readme details about the task queue

* Adding notes about partition sizes

* Improve note about how schedulign tasks works

---------

Co-authored-by: Vimal Jobanputra <[email protected]>

* Fixup the path being passed to archive build reports (#99)

* Adding in sample data scripts and way to run from gradle (#101)

* Adding in sample data scripts and way to run from gradle

* Update so source id is unique per row

* Update to insert observation data for 50 sites

* Adding in a simple script to add time to the observed data

* Tweak how loading is done to avoid the function

* Update the refresh to be relative to current date

* Adding in transactions for the scripts

* Bump msw from 1.2.1 to 1.3.2 in /packages/PlanLimitsUI (#108)

Bumps [msw](https:/mswjs/msw) from 1.2.1 to 1.3.2.
- [Release notes](https:/mswjs/msw/releases)
- [Changelog](https:/mswjs/msw/blob/main/CHANGELOG.md)
- [Commits](mswjs/msw@v1.2.1...v1.3.2)

---
updated-dependencies:
- dependency-name: msw
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump net.logstash.logback:logstash-logback-encoder (#107)

Bumps [net.logstash.logback:logstash-logback-encoder](https:/logfellow/logstash-logback-encoder) from 7.3 to 7.4.
- [Release notes](https:/logfellow/logstash-logback-encoder/releases)
- [Commits](logfellow/logstash-logback-encoder@logstash-logback-encoder-7.3...logstash-logback-encoder-7.4)

---
updated-dependencies:
- dependency-name: net.logstash.logback:logstash-logback-encoder
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Add query and controller endpoint for usage data (#102)

* Weekly overview table (#124)

* Minor code cleanups/comments

* Move non-component files into lib, and move sidebar into a subdir

* Remove redundant files

* Extract Sidebar components

* Wire up basic loading mechanism

* Copy across button and indicator components from Interrain

* Extract hook to load and transform data, start formatting usage table

* Add GroundWater and populate tooltips with the right data

* Take into account WaterTakeFilter when showing usage table

* Clean up some naming

* Ensure WaterAllocationMessage messages match IngestAPI types

* Added fields to water usage API required by the front-end

* Camelise API properties

* Ensure usage dates are sorted

* Better handling of errors loading usage data

* Feedback from code review

* Moving WaterAllocation View to a Materialised View (#125)

* Moving WaterAllocation View to a Materialised View

* Remove spots

* Updates to allow water_allocation_and_usage_by_area to be refreshed

* Fix up permission issue with materialized view

* Turn effective_daily_consents back into a regular migration so we can ensure it is run before water_allocation_and_usage_by_area

* Add materialized_views_role definition to LocalInfrastructure init script

* Ensure materialized view is refreshed before running tests

* Ensure materialized_views_role has the same permissions as eop_manager_app_user

* Ensure view is materialized after generating test data

* Remove old version of DB config

- Has been superceded by LocalInfra folder

* Limit materialized views role to Read only

---------

Co-authored-by: Vim <[email protected]>

* Update plan limit libs (#127)

* Update react-router-dom

* Update Vite and associated packages

* Update typescript and related packages, sort dev and main packages

* Update linting and formatting libs

And remove pretty-quick which is not compatible with prettier 3: prettier/pretty-quick#164

* Update testing libs

* Update autoprefixer, remove unused web-vitals

* Update tailwind, postcss and related libs

* Update data/state libraries

* npm audit fix

* Update react-map-gl and maplibre-gl

* Update MSW

* Moving the materalized view permissions into main migrations (#129)

* Move permissions required for materialized_views to migration

* Add RESET ROLE to script

- Otherwise the connection goes back to the pool with the wrong role,
  and causes pain for later threads.

* Fix permissions so app can query usage view (#137)

* Config ErrorHandlingDeserializer to DLQ unparsable messages (#134)

* Re-apply changes from PR #130 (#138)

* Changes for Water Use View (#139)

* Changes for Water Use View

- Move data to NZST timezone when aggregating daily
- Fix issue with Water Meter Volume, previously we were treating it as a
  l/s value when it is actually a raw M^3 used since the last data
  point. Which makes the aggregation a simple sum

* Update test with new data

* Weekly usage graph (#148)

* WIP: Detailed usage page with initial weekly usage

* WIP: Group heatmap into regions and order

* WIP: Tidy up presentation and data

* Use full path in link to detailed usage page to fix AWS Amplify weirdness

* Display tweaks

* Add a link back to the limits viewer

* Error handling and code cleanup

* Fix bug formatting date in tooltip

* Make UsageDisplayGroups part of council data

* Crawler fix start of month (#149)

* Tweak to limit amount of history kept.

* Extend measurement list so it will be on month ahead if there are recent
measurements

* Add new fetch task that is constantly refreshing the latest data

* Refactor to make comparsion more explicit

* Update README for recent changes

* Use correct allocation measure (#151)

* Bump @typescript-eslint/parser in /packages/PlanLimitsUI (#140)

Bumps [@typescript-eslint/parser](https:/typescript-eslint/typescript-eslint/tree/HEAD/packages/parser) from 6.9.0 to 6.10.0.
- [Release notes](https:/typescript-eslint/typescript-eslint/releases)
- [Changelog](https:/typescript-eslint/typescript-eslint/blob/main/packages/parser/CHANGELOG.md)
- [Commits](https:/typescript-eslint/typescript-eslint/commits/v6.10.0/packages/parser)

---
updated-dependencies:
- dependency-name: "@typescript-eslint/parser"
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump maplibre-gl from 3.5.1 to 3.5.2 in /packages/PlanLimitsUI (#132)

Bumps [maplibre-gl](https:/maplibre/maplibre-gl-js) from 3.5.1 to 3.5.2.
- [Release notes](https:/maplibre/maplibre-gl-js/releases)
- [Changelog](https:/maplibre/maplibre-gl-js/blob/main/CHANGELOG.md)
- [Commits](maplibre/maplibre-gl-js@v3.5.1...v3.5.2)

---
updated-dependencies:
- dependency-name: maplibre-gl
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Daily Usage Graph (#158)

* Extract results to separate components

* Display daily usage

* Use the same color scheme for daily usage we weekly usage

* Format data in tooltip

* Add a key to the daily usage chart

* Add legend to weekly usage

* Tweaks based on feedback

* Fix rounding issue

* Test alternate visualisation type

* Add a key to the heatmapped daily data

* Code tidy-ups

* Remove TimeRange graph and tidy up code

* Remove unused renderCell Override

* Ensure top axis is not generated for weekly graphs other than first

* Copy tweaks

* Water meter reading view bugfix (#159)

* fix for bug in daily usage calc

* remove date filter.

no longer needed as filtering occurs on a downstream view

* Handle missing data in the UI (#165)

* Handle missing data for Usage table

* Handle missing data in Detailed Daily usage graph

* Display tweaks

* Handle missing and show more detailed info for weekly usage

* Formatting tweaks

* Only show extended tooltip data when the d key is held down

* Remove console.log

* Formatting tweaks

* More explicit handling of 0 vs NULL values when displaying usage

* Always show daily usage table in weekly tooltip

* Show CMU if CMSU is not present

* Handle missing data in view (#173)

* Update field names to match updated view

* Don't default to 0's where NULLs are present in Usage and Allocation data

* Update view with showing allocation vs usage

* Update view tests

---------

Co-authored-by: Vim <[email protected]>

* Add overview docs (#174)

* Add some docs

* Docs tweaks

* Remove log error message about rack assigner

* Adding date check back in because of how it effect materlizing the view (#161)

* Lower number of consumers for Kafka to hopefully stabalise things

* Update Jackson max message size

* Crawler handle large messages (#225)

* Skip large messages from Hilltop

- Some messages will cause issues later in processing because of their
  size

* Fixup off by one day error in test

* Update manager so observation updated_at is only changed if the value
changes

- This is better information, but also should imporve loading
  performance

* TEAMU-876 - Show what percentage of allocation is measured (#232)

* Update Ingest API for new message format

* Update manager with new fields

* Update Plan Limits with new data

* Adding in Measured vs Total Allocation for usage

* https://gwrc.atlassian.net/browse/TEAMU-929 : Daily Aggregated Observations Multiple values for same day (#235)

* Update Ingest API for new message format

* Update manager with new fields

* Update Plan Limits with new data

* Adding in Measured vs Total Allocation for usage

* added the .sql file which removes the daily duplicates for each site

* fix the where clause position

* drop the view and create it as there are structural changes

* drop the view and create it as there are structural changes

* drop the cascaded view and recreate them

* removed the drop command

* removed the drop command

* changes to ALTER view

* changes to ALTER view

* added drop and removed alter

* added both view script in one migration file

* added both view script in one migration file

* added both view script in one migration file

* added both view in the same migration file and corrected the sql query

* keeping the naming conventions same

* Adjusting roles to allow removeing the materalised view

---------

Co-authored-by: Steve Mosley <[email protected]>

* TEAMU-1016-Fix-leap-year-issue (#243)

* TEAMU-1016 Add FE and BE unit tests

* TEAMU-1016 Add FE and BE unit tests

* TEAMU-1016 Improve BE exception handling

* TEAMU-1016 Teak start scripts

* TEAMU-1016 Check error message in test

* TEAMU-1016 Lint fixes

* TEAMU-1016 Spelling error

* TEAMU-1016 Linting and batect run

* TEAMU-1016 Resolve merge conflict in readme

---------

Co-authored-by: wtcarter-gw <[email protected]>

* TEAMU-940 Add new migration 41 replacing MAX and MIN with last and first (#248)

* TEAMU-940 Add new migration 41

* TEAMU-940 Correct migration 41 as per failing test

* TEAMU-940 Apply review feedback and add some tests

* TEAMU-940 Augment tests

* TEAMU-940 Spotless linting

---------

Co-authored-by: wtcarter-gw <[email protected]>

* fix to exclude all combined_meters (#249)

* fix to exclude all combined_meters

* Remove the extra space

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: Vimal Jobanputra <[email protected]>
Co-authored-by: Martin Peak <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Swapna Josmi Sam <[email protected]>
Co-authored-by: Wayne Carter <[email protected]>
Co-authored-by: wtcarter-gw <[email protected]>
wtcarter pushed a commit that referenced this pull request Apr 10, 2024
@smozely
Config ErrorHandlingDeserializer to DLQ unparsable messages (#134)
0ae108f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vimto vimto vimto approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@smozely @vimto