OpenSearchAdapter - Include field name in error log about map update failure

[baroquebobcat]

Summary

When the OpenSearchAdapter has an error doing a map update for a field, it doesn't include the field name in the log message. This adds the field name to the log message.

Requirements

• I've read and understood the Contributing Guidelines and have done my best effort to follow them.
• I've read and agree to the Code of Conduct.

[bryanlb]

@baroquebobcat I'm not sure this is needed? The current field information should already be captured in the exception message, and looks something like the following:

Error doing map update errorMsg=can't merge a non object mapping [request_body] with an object mapping

Error doing map update errorMsg=can't merge a non object mapping [upstream_cluster] with an object mapping 

[baroquebobcat]

I'm not sure this is needed?
That's fair. I don't feel that strongly about it. I added this while debugging because it wasn't immediately clear to me that the [] in the error was the field name and because it's a Exception catch which could catch other types of exceptions that may have less context in them.

I was also trying to align it with the behavior for entries above
https://github.com/slackhq/astra/blob/master/astra/src/main/java/com/slack/astra/logstore/opensearch/OpenSearchAdapter.java#L283

Maybe it would make sense to break this out into a catch for the expected error cases that just includes the message, and another for unexpected that includes more info, like

catch (IllegalArgumentException | MapperException e) {
  LOG.error("Error doing map update for {}, errorMsg={}", fieldName, e.getMessage());
} catch (Exception e) {
  LOG.error("Error doing map update for {}", fieldName, e);
}

But I'm not sure that's necessary here.

Oh, do you know if the fieldname in the error from the merge call will include the full path in the case where it refers to a nested field?

[bryanlb]

@baroquebobcat I think your proposed splitting of the stack trace into different error messages would be useful, because as you mentioned the catch-all exception handling could have other root causes.

I'm not sure what the exception looks like when it's a nested field. I currently have some work-in-progress that reworks the schema initialization/mapper for the caches nodes in #1052 (here), that does the registration in a single shot instead of field-by-field. The benefit of that is you completely avoid these nested conflicts, and it's significantly faster to load.

I haven't explored if that approach can be adapted for the indexer yet though.