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 support for es-tags-as-fields through env #105

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jam01
Copy link

@jam01 jam01 commented Jan 26, 2021

Which problem is this PR solving?

Short description of the changes

  • The elasticsearch SpanDeserializer will replace tag keys through environment variables configuration.
  • For the test I'm starting and closing a spark context manually and loading spans from a file, which avoids requiring a live elasticsearch instance.

@jam01 jam01 force-pushed the fix/es-tags-as-fields branch from 3137223 to ec9e957 Compare January 26, 2021 22:37
Comment on lines +55 to +57
Field field = env.getClass().getDeclaredField("m");
field.setAccessible(true);
((Map<String, String>) field.get(env)).put(name, val);
Copy link
Contributor

Choose a reason for hiding this comment

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

This does not look right, and it will likely make the maintenance harder.

Can we have an alternative way to pass configuration so we don't have to override private fields in java.lang namespace?

@@ -38,9 +43,28 @@ public class SpanDeserializer extends StdDeserializer<Span> {

// TODO Spark incorrectly serializes object mapper, therefore reinitializing here
private ObjectMapper objectMapper = JsonHelper.configure(new ObjectMapper());
private final boolean tagsAsFieldsAll;
private final List<String> tagsAsFields;
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you considered Set<String>?

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

Successfully merging this pull request may close these issues.

2 participants