Compound reg parent audit log

[jmcneil86043]

A long-time ACAS user needs an audit-log for any changes to the small-molecule parent. This is for IP reasons. I believe the because structure changes are an actual IP record they properly belong in the schema, and not just captured by service monitoring. The parent table has an ignored column that could be used to save old records, but it has not been used in practice. I am concerned that adding ignored records could have unintended consequences in both ACAS code (which could be fixed) and also in other tools like Data Integrator and database views that clients may have created. An uglier, but less risky approach would be to make a parent archive table.

The CReg schema was intentionally designed to be easy to query for chemists and therefore has a more conventional form than the rest of ACAS. For this reason, I am not very concerned about ignoring ACAS standards (pun intended).

I propose these steps:

• Create a new parent_audit table with db migration to add it on upgrade
• Modify the parent update function to optionally copy previous to audit table
  • Possible good place to trap these changes:
    • acas-roo-server/src/main/java/com/labsynch/labseer/service/ParentServiceImpl.java
      • public Collection updateParent(Parent parent) throws NonUniqueAliasException {
  • option to archive audit should be added to compound reg's configuration
• Make API to return JSON history of structure by corp_name

Is there are better approach? The client would sponsor this enhancement to OSS ACAS/Creg

[bffrost]

Interesting! I think the industry term for this topic is Change Data Capture (CDC)
https://www.striim.com/blog/change-data-capture-cdc-what-it-is-and-how-it-works/
https://www.qlik.com/us/change-data-capture/cdc-change-data-capture

That first link above outlines several methods:

• Audit columns (our ignored and modified dates are examples of this)
• Table Deltas
• Trigger-based CDC
• Log-based CDC

I don't like Table Deltas for this use case, but I think any of the other 3 could be considered.

Trigger-based CDC would mean adding database triggers in Postgres to write to the "shadow tables" or "audit tables" as you've called them. This could involve a performance hit, but it'd be in the database layer so it'd likely be less of a hit than doing the same thing in Java. We do also already have SQL knowledge and a robust way of adding SQL migrations through flyway.

Log-based CDC would read the changes from the database logs and stream them elsewhere (or even back into an archive / audit table). One stack to do this would be Debezium + Kafka
https://debezium.io/documentation/reference/stable/architecture.html

Pros:

• Opens up a broad category of possible improvements for cascading changes or alerts to different parts of ACAS, or eventually even doing away with the "ignored" standard in ACAS.
• No impact on existing logic & performance since it would read from Postgres logs
  Cons:
• Introduces a whole new set of technologies to learn, deploy, and maintain
• Probably overcomplicated for one-off changes like the Update Parent mentioned above, but could start to pay for itself if we need to do this more and more for different data types (Parents, Lots, Experiments, Analysis Groups, Containers...), or if we wanted to expand to different use cases for change data capture besides audit logs.

[jmcneil86043]

Hi Brian, thanks for digging into this in detail! I would argue this is not a CDC requirement, but rather for maintaining an auditable change log for IP and regulatory purposes. Data Integrator is CDC that uses the trigger option. That said, your technical analysis is on point.

I agree that table deltas are not the way to go. As you said, ACAS has the infrastructure to do this with SQL triggers. Our original design philosophy was to be as database agnostic as possible, which is why we could support Oracle so easily. Using triggers would be a step away from that philosophy. It may be a good step, but I want to highlight it. I'd reject log-based approaches since they are much more database-specific than triggers, which are at least conceptually similar across databases.

This thinking is why I proposed doing it in the application logic ( a variation of the first option)

I lean to triggers because we could build them and test them independently for ACAS logic, and that offers a layer or robustness. This may be because we just uncovered a gap in the ACAS assay data reg change logging at another client, and no one noticed for 4 years - until they needed the history.

[bffrost]

Hi John. That makes sense, and I agree it's not a CDC requirement, although the technology does seem closely related.

re: tying to databases, we do already have specific schema implementations for Postgres and Oracle, and both databases support triggers, so it does seem viable to create a Postgres and an Oracle flyway migration for each trigger to maintain cross-compatibility. It'd be good to avoid Postgres-specific SQL features, but I imagine "insert the old row elsewhere every time the row is updated" is pretty doable with standard SQL.

I'd push back a little on log-based approaches. While you're right that they are much more database-specific, there are technologies out there like Debezium that have connectors for both Oracle and Postgres (https://debezium.io/documentation/reference/stable/connectors/index.html) so I'd argue using tech like that would actually move us further from database lock-in.

I lean towards triggers too, and the independence is a great point. It seems important to build up both some human process and some automation to regularly QA/QC the audit trail / audit tables, and it'd be great if those didn't follow the normal ACAS codepaths either.

[jmcneil86043]

We are going to write a trigger-based solution and contribute to the OSS project when we get it working

[brianbolt]

@philippcheung - @jmcneil86043 mentioned you implemented the triggers and have them and can share them with us. Can you upload here or create a PR? Thanks!

[philippcheung]

For future reference @jmcneil86043 -- it can be found on bug NIM-616 in our internal tracking system.

Here's the trigger we deployed.

-- root@d8fbb7877f6b:/bingo-build# pg_dump --table parent --schema-only -Uacas

drop table if exists acas.parent_audit;

CREATE TABLE acas.parent_audit (
    id serial not null primary key,
	  parent_id bigint NOT NULL,
    cd_id integer NOT NULL,
    common_name character varying(1000),
    corp_name character varying(255) NOT NULL,
    ignore boolean,
    mol_formula character varying(4000),
    mol_structure text,
    mol_weight double precision,
    parent_number bigint NOT NULL,
    registration_date timestamp without time zone,
    stereo_comment character varying(1000),
    version integer,
    chemist character varying,
    stereo_category bigint,
    modified_date timestamp without time zone,
    modified_by character varying,
    bulk_load_file bigint,
    parent_annotation bigint,
    compound_type bigint,
    comment text,
    exact_mass double precision,
    registered_by character varying,
    is_mixture boolean,
    audit_date timestamp without time zone,
    changed_by character varying
);


ALTER TABLE acas.parent_audit OWNER TO acas;

-- ALTER TABLE ONLY acas.parent_audit ADD CONSTRAINT parent_audit_pkey PRIMARY KEY (id);

CREATE INDEX parent_audit_cdid_idx ON acas.parent_audit USING btree (cd_id);

CREATE INDEX parent_audit_parentid_idx ON acas.parent_audit USING btree (parent_id);

CREATE INDEX parent_audit_corpname_idx ON acas.parent_audit USING btree (corp_name);


-- Create the store procedure

CREATE OR REPLACE FUNCTION log_parent_changes()
  RETURNS TRIGGER 
  LANGUAGE PLPGSQL
  AS
$$
BEGIN
	IF 
		NEW.mol_structure <> OLD.mol_structure OR 
		NEW.stereo_category <> OLD.stereo_category OR 
		NEW.stereo_comment <> OLD.stereo_comment OR 
		-- NEW.alias <> OLD.alias OR 
		NEW.is_mixture <> OLD.is_mixture 
	THEN
		 INSERT INTO parent_audit
		 	(parent_id, cd_id, common_name, corp_name,
			 ignore, mol_formula, mol_structure, mol_weight,
			 parent_number, registration_date, stereo_comment,
			 version, chemist, stereo_category, modified_date,
			 modified_by, bulk_load_file, parent_annotation, compound_type,
			 comment, exact_mass, registered_by, is_mixture,
			 audit_date, changed_by)
		 VALUES
		 	(OLD.id, OLD.cd_id, OLD.common_name, OLD.corp_name,
			 OLD.ignore, OLD.mol_formula, OLD.mol_structure, OLD.mol_weight,
			 OLD.parent_number, OLD.registration_date, OLD.stereo_comment,
			 OLD.version, OLD.chemist, OLD.stereo_category, OLD.modified_date,
			 OLD.modified_by, OLD.bulk_load_file, OLD.parent_annotation, OLD.compound_type,
			 OLD.comment, OLD.exact_mass, OLD.registered_by, OLD.is_mixture,
			 NEW.modified_date, NEW.modified_by);
	END IF;

	RETURN NEW;
END;
$$;

DROP TRIGGER IF EXISTS parent_changes on parent;

CREATE TRIGGER parent_changes
  BEFORE UPDATE
  ON parent
  FOR EACH ROW
  EXECUTE PROCEDURE log_parent_changes();

[brianbolt]

Thanks!