Skip to content

Commit

Permalink
Improve operation execution reporting
Browse files Browse the repository at this point in the history
1) Added "redirection" item in the record so that we can store
orphaned failures in backup objects (usually tasks).

2) Implemented redirection for reporting in iterative tasks.

3) Setting up (emergency) shadow name for malformed shadows.
  • Loading branch information
mederly committed Feb 2, 2021
1 parent 42848af commit 9483dbd
Show file tree
Hide file tree
Showing 13 changed files with 220 additions and 40 deletions.
Expand Up @@ -28475,6 +28475,17 @@
</xsd:appinfo>
</xsd:annotation>
</xsd:element>
<xsd:element name="redirection" type="tns:OperationExecutionRecordRedirectionType" minOccurs="0">
<xsd:annotation>
<xsd:documentation>
Information about operation execution record redirection: the fact that the record
belongs to object other than its current holder.
</xsd:documentation>
<xsd:appinfo>
<a:since>4.3</a:since>
</xsd:appinfo>
</xsd:annotation>
</xsd:element>
<xsd:element name="timestamp" type="xsd:dateTime" minOccurs="0">
<xsd:annotation>
<xsd:documentation>
Expand Down Expand Up @@ -28569,6 +28580,47 @@
</xsd:restriction>
</xsd:simpleType>

<xsd:complexType name="OperationExecutionRecordRedirectionType">
<xsd:annotation>
<xsd:documentation>
Information about operation execution record redirection: the fact that the record
belongs to object other than its current holder.
</xsd:documentation>
<xsd:appinfo>
<a:since>4.3</a:since>
<a:experimental>true</a:experimental>
<a:container>true</a:container>
</xsd:appinfo>
</xsd:annotation>
<xsd:sequence>
<xsd:element name="redirected" type="xsd:boolean" minOccurs="0">
<xsd:annotation>
<xsd:documentation>
True if the operation execution record belongs to object other than the current one.
(Actually this flag exists solely to allow easy query formulation and implementation.
TODO: reconsider it.)
</xsd:documentation>
</xsd:annotation>
</xsd:element>
<xsd:element name="objectType" type="xsd:QName" minOccurs="0">
<xsd:annotation>
<xsd:documentation>
Type of the proper record holder (if known).
</xsd:documentation>
</xsd:annotation>
</xsd:element>
<xsd:element name="identification" type="xsd:string" minOccurs="0">
<xsd:annotation>
<xsd:documentation>
Identification of the proper record holder. Free-form, using
any relevant data that is available. An example: connector object UID.
</xsd:documentation>
</xsd:annotation>
</xsd:element>
</xsd:sequence>
</xsd:complexType>
<xsd:element name="operationExecutionRecordRedirection" type="tns:OperationExecutionRecordRedirectionType"/>

<xsd:complexType name="LocalizableMessageType" abstract="true">
<xsd:annotation>
<xsd:documentation>
Expand Down
Expand Up @@ -7,15 +7,17 @@

package com.evolveum.midpoint.model.impl.sync.tasks;

import org.jetbrains.annotations.NotNull;

import com.evolveum.midpoint.provisioning.api.ResourceObjectShadowChangeDescription;
import com.evolveum.midpoint.provisioning.api.SynchronizationEvent;
import com.evolveum.midpoint.repo.common.task.AbstractIterativeItemProcessor;
import com.evolveum.midpoint.repo.common.task.ItemProcessingRequest;
import com.evolveum.midpoint.repo.common.util.OperationExecutionRecorderForTasks;
import com.evolveum.midpoint.schema.result.OperationResult;
import com.evolveum.midpoint.schema.statistics.IterationItemInformation;
import com.evolveum.midpoint.xml.ns._public.common.common_3.ShadowType;

import org.jetbrains.annotations.NotNull;
import com.evolveum.midpoint.xml.ns._public.common.common_3.TaskType;

/**
* TODO
Expand All @@ -36,15 +38,21 @@ public Object getCorrelationValue() {
}

@Override
public ShadowType getObjectToReportOperation() {
public OperationExecutionRecorderForTasks.Target getOperationExecutionRecordingTarget() {
ResourceObjectShadowChangeDescription changeDescription = getItem().getChangeDescription();
if (changeDescription != null && changeDescription.getCurrentShadow() != null) {
return changeDescription.getCurrentShadow().asObjectable(); // TODO
return createRecordingTargetForObject(changeDescription.getCurrentShadow());
} else {
return null;
return new OperationExecutionRecorderForTasks.Target(null, ShadowType.COMPLEX_TYPE,
getMalformedEventIdentification(), getRootTaskOid(), TaskType.class);
}
}

/** TODO (eventually also move to SynchronizationEvent) */
private String getMalformedEventIdentification() {
return getItem().toString();
}

@Override
public String getObjectOidToRecordRetryTrigger() {
SE event = getItem();
Expand Down
Expand Up @@ -183,9 +183,9 @@ public void test110ImportWithSingleMalformedAccount() throws Exception {
.display()
.assertSuccessCount(9)
.assertFailureCount(1)
.assertLastFailureObjectName(null) // TODO it should be reported in a better way
.assertLastFailureObjectName("u-000001")
.end();
// TODO the error should be somehow reported in the task (currently it is not)
// TODO assert redirected errors in the task
}

@Test
Expand All @@ -212,7 +212,7 @@ public void test120ImportWithAllFailuresEnabled() throws Exception {
.assertSuccessCount(8)
.assertFailureCount(2)
.end();
// TODO the error should be somehow reported in the task (currently it is not)
// TODO assert redirected errors in the task

assertShadow(formatAccountName(IDX_GOOD_ACCOUNT), RESOURCE_DUMMY_SOURCE.getResource())
.display();
Expand Down Expand Up @@ -250,5 +250,6 @@ public void test130ReconciliationWithAllFailuresEnabled() throws Exception {
.display();
assertShadow(formatAccountName(IDX_PROJECTOR_FATAL_ERROR), RESOURCE_DUMMY_SOURCE.getResource())
.display();
// TODO assert redirected errors in the task
}
}
Expand Up @@ -2082,6 +2082,7 @@ private SearchResultMetadata searchObjectIterativeResource(ProvisioningContext c
resultShadow = resourceObject;
LOGGER.error("An error occurred while processing resource object {}. Recording it into object fetch result: {}",
resourceObject, t.getMessage(), t);
setShadowNameInEmergency(resultShadow);
ObjectTypeUtil.recordFetchError(resultShadow, objResult);
} else {
// This is the default (4.2 and before) behavior: we silently skip the problematic object and stop.
Expand Down Expand Up @@ -2131,6 +2132,19 @@ private SearchResultMetadata searchObjectIterativeResource(ProvisioningContext c
}
}

private void setShadowNameInEmergency(PrismObject<ShadowType> shadow) {
if (shadow.asObjectable().getName() == null) {
try {
PolyString name = ShadowUtil.determineShadowName(shadow);
if (name != null) {
shadow.asObjectable().setName(new PolyStringType(name));
}
} catch (SchemaException e) {
LOGGER.warn("Couldn't determine the name for {}", shadow, e);
}
}
}

/**
* Contains processing of an object that has been found on a resource before it is passed to the caller-provided handler.
* We do basically four things:
Expand Down
Expand Up @@ -60,10 +60,10 @@ class ConnIdConvertor {

/**
* Converts ICF ConnectorObject to the midPoint ResourceObject.
* <p/>
*
* All the attributes are mapped using the same way as they are mapped in
* the schema (which is actually no mapping at all now).
* <p/>
*
* If an optional ResourceObjectDefinition was provided, the resulting
* ResourceObject is schema-aware (getDefinition() method works). If no
* ResourceObjectDefinition was provided, the object is schema-less. TODO:
Expand Down
Expand Up @@ -135,8 +135,8 @@ public String convertAttributeNameToConnId(PropertyDelta<?> attributeDelta, Obje
throws SchemaException {
PrismPropertyDefinition<?> propDef = attributeDelta.getDefinition();
ResourceAttributeDefinition attrDef;
if (propDef != null && (propDef instanceof ResourceAttributeDefinition)) {
attrDef = (ResourceAttributeDefinition)propDef;
if (propDef instanceof ResourceAttributeDefinition) {
attrDef = (ResourceAttributeDefinition) propDef;
} else {
attrDef = ocDef.findAttributeDefinition(attributeDelta.getElementName());
if (attrDef == null) {
Expand Down
Expand Up @@ -26,6 +26,7 @@
import com.evolveum.midpoint.xml.ns._public.common.common_3.LockoutStatusType;
import com.evolveum.midpoint.xml.ns._public.common.common_3.ShadowType;

import com.evolveum.prism.xml.ns._public.types_3.PolyStringType;
import com.evolveum.prism.xml.ns._public.types_3.ProtectedStringType;

import org.identityconnectors.common.security.GuardedString;
Expand Down Expand Up @@ -409,8 +410,10 @@ private ResourceAttributeDefinition<Object> findAttributeDefinition(String connI
* @param result Operation result reflecting the failure.
*/
@NotNull PrismObject<ShadowType> reportErrorInFetchResult(OperationResult result) {
// TODO consider treatment of an empty resource object (e.g. by creating a fake name containing identifier values)
ObjectTypeUtil.recordFetchError(resourceObject, result);
if (resourceObject.asObjectable().getName() == null) {
resourceObject.asObjectable().setName(PolyStringType.fromOrig(connectorObject.getUid().getUidValue()));
}
return resourceObject;
}
}
Expand Up @@ -64,4 +64,8 @@ protected AbstractIterativeItemProcessor(@NotNull PE partExecution) {
*/
public abstract boolean process(ItemProcessingRequest<I> request, RunningTask workerTask, OperationResult parentResult)
throws CommonException, PreconditionViolationException;

public @NotNull TE getTaskExecution() {
return taskExecution;
}
}
Expand Up @@ -218,4 +218,8 @@ public void setPermanentErrorEncountered(@NotNull Throwable reason) {
public @NotNull ErrorState getErrorState() {
return errorState;
}

public @NotNull String getRootTaskOid() {
return localCoordinatorTask.getRootTaskOid();
}
}
Expand Up @@ -22,7 +22,6 @@
import com.evolveum.midpoint.util.exception.SchemaException;
import com.evolveum.midpoint.util.logging.*;
import com.evolveum.midpoint.xml.ns._public.common.common_3.CriticalityType;
import com.evolveum.midpoint.xml.ns._public.common.common_3.ObjectType;
import com.evolveum.midpoint.xml.ns._public.common.common_3.TaskPartitionDefinitionType;
import com.evolveum.midpoint.xml.ns._public.common.common_3.TracingRootType;

Expand Down Expand Up @@ -360,13 +359,10 @@ private void writeOperationExecutionRecord(OperationResult result) {
return;
}

ObjectType objectToReportOperation = request.getObjectToReportOperation();
if (objectToReportOperation != null) {
getOperationExecutionRecorder().recordOperationExecution(objectToReportOperation.asPrismObject(), resultException,
taskExecution.localCoordinatorTask, result);
} else {
// TODO record the operation somewhere else (to the task, probably)
}
OperationExecutionRecorderForTasks.Target target = request.getOperationExecutionRecordingTarget();
RunningTask task = taskExecution.localCoordinatorTask;

getOperationExecutionRecorder().recordOperationExecution(target, resultException, task, result);
}

private OperationExecutionRecorderForTasks getOperationExecutionRecorder() {
Expand Down
Expand Up @@ -7,14 +7,24 @@

package com.evolveum.midpoint.repo.common.task;

import com.evolveum.midpoint.prism.PrismContext;
import com.evolveum.midpoint.prism.PrismObject;
import com.evolveum.midpoint.repo.common.util.OperationExecutionRecorderForTasks;
import com.evolveum.midpoint.schema.AcknowledgementSink;
import com.evolveum.midpoint.schema.result.OperationResult;
import com.evolveum.midpoint.schema.statistics.IterationItemInformation;
import com.evolveum.midpoint.task.api.RunningTask;

import com.evolveum.midpoint.xml.ns._public.common.common_3.ObjectType;
import com.evolveum.midpoint.xml.ns._public.common.common_3.TaskType;

import org.jetbrains.annotations.NotNull;

import javax.xml.namespace.QName;
import java.util.Objects;

import static com.evolveum.midpoint.prism.polystring.PolyString.getOrig;

/**
* <p>Holds an item that is scheduled for processing.</p>
*
Expand Down Expand Up @@ -43,24 +53,38 @@ public Object getCorrelationValue() {
}

/**
* @return Object to which we will write an operation execution record. If null or not existing,
* an alternative target is to be found.
* @return Object to which we will write an operation execution record (plus auxiliary information).
*/
public abstract ObjectType getObjectToReportOperation();
public abstract OperationExecutionRecorderForTasks.Target getOperationExecutionRecordingTarget();

@NotNull
protected OperationExecutionRecorderForTasks.Target createRecordingTargetForObject(PrismObject<? extends ObjectType> object) {
return new OperationExecutionRecorderForTasks.Target(object, getType(object), getOrig(object.getName()),
getRootTaskOid(), TaskType.class);
}

/**
* @return OID of object to which we put a trigger causing operation retry (if known)
*/
public abstract String getObjectOidToRecordRetryTrigger();

public void done() {
// TODO
}

public abstract @NotNull IterationItemInformation getIterationItemInformation();

public boolean process(RunningTask workerTask, OperationResult result) {
ItemProcessingGatekeeper<I> administrator = new ItemProcessingGatekeeper<>(this, itemProcessor, workerTask);
return administrator.process(result);
}

protected PrismContext getPrismContext() {
return itemProcessor.taskExecution.getPrismContext();
}

protected @NotNull String getRootTaskOid() {
return itemProcessor.getTaskExecution().getRootTaskOid();
}

protected @NotNull QName getType(PrismObject<?> object) {
return Objects.requireNonNull(
getPrismContext().getSchemaRegistry().determineTypeForClass(object.getCompileTimeClass()));
}
}
Expand Up @@ -7,6 +7,7 @@

package com.evolveum.midpoint.repo.common.task;

import com.evolveum.midpoint.repo.common.util.OperationExecutionRecorderForTasks.Target;
import com.evolveum.midpoint.schema.result.OperationResult;

import org.jetbrains.annotations.NotNull;
Expand All @@ -30,8 +31,8 @@ public class ObjectProcessingRequest<O extends ObjectType> extends ItemProcessin
}

@Override
public ObjectType getObjectToReportOperation() {
return getItem().asObjectable(); // TODO
public Target getOperationExecutionRecordingTarget() {
return createRecordingTargetForObject(getItem());
}

@Override
Expand Down

0 comments on commit 9483dbd

Please sign in to comment.