Skip to content

Fixed NullpointerException when no fields are set on processors.#305

Closed
jgogstad wants to merge 1 commit into
microsoft:masterfrom
jgogstad:processor_configuration_errors
Closed

Fixed NullpointerException when no fields are set on processors.#305
jgogstad wants to merge 1 commit into
microsoft:masterfrom
jgogstad:processor_configuration_errors

Conversation

@jgogstad

@jgogstad jgogstad commented Aug 8, 2016

Copy link
Copy Markdown
Contributor

The current implementation of TelemetryConfigurationFactory throws NullPointerException whenever it encounters processors that are not configured with any attributes, e.g.

<ApplicationInsights>
   <TelemetryProcessors>
     <CustomProcessors>
        <Processor type="com.example.MyProcessor"/>
    </CustomProcessors>
 </TelemetryProcessors>
</ApplicationInsights>

There's also a bug where the same processor instance is added multiple times. This occurs when multiple fields are set on a processor.

This PR fixes both (and includes PR #304 ).

…d bug where processor would be added multiple times when several fields are set.
@msftclas

msftclas commented Aug 8, 2016

Copy link
Copy Markdown

Hi @jgogstad, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!

This seems like a small (but important) contribution, so no Contribution License Agreement is required at this point. Real humans will now evaluate your PR.

TTYL, MSBOT;

@msftclas

msftclas commented Aug 8, 2016

Copy link
Copy Markdown

@jgogstad, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, MSBOT;

@gupele

gupele commented Aug 9, 2016

Copy link
Copy Markdown
Contributor

@jgogstad, thanks for your valuable input.

We will not use this PR since we have an ongoing background effort to refactor the TC Factory, I already started doing that for the TelemetryProcessor, I will use your input for the PR I am going to submit soon

Thanks!

@jgogstad

jgogstad commented Aug 9, 2016

Copy link
Copy Markdown
Contributor Author

Brilliant! We'll use a fork until it's ready. Looking forward to see the result. Closing this.

@jgogstad jgogstad closed this Aug 9, 2016
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.

3 participants