Make ElasticAPM.report return an error or error id instead of true

greyblake · August 23, 2019, 11:03am

APM Agent language and version: Ruby, 2.9.1

Current status

Hi, currently ElasticAPM.report and ElasticAPM.report_message return either nil or true

ElasticAPM.report(exception)
=> true

Proposal

I would like to propose a change to make it return an instance ofElasticAPM::Error instead. Or at least error.id.

Motivation

The reason for this, we'd like to be able to communicate an error id back to a client as part of server error payload. This would allow us to easily identify errors in ELK by id.

Required code change

The code change to achieve this is very small.
We just need to modify ElasticAPM.report and ElasticAPM.report_message methods

github.com

elastic/apm-agent-ruby/blob/master/lib/elastic_apm/agent.rb#L198


# errors


def report(exception, context: nil, handled: true)
  return if config.filter_exception_types.include?(exception.class.to_s)


  error = @error_builder.build_exception(
    exception,
    context: context,
    handled: handled
  )
  enqueue error
end


def report_message(message, context: nil, backtrace: nil, **attrs)
  error = @error_builder.build_log(
    message,
    context: context,
    backtrace: backtrace,
    **attrs
  )
  enqueue error

so they return error, instead of enqueue error.

What do you think?
If you give me a green light I can prepare a pull request for this.

Thanks in advance.

felixbarny · August 23, 2019, 11:23am

I'm not in charge of the Ruby agent but I think it's a great idea which would also work well in the Java agent.

mikker · August 26, 2019, 8:15am

Hi Sergey!

I'm not opposed to that. I've opened this PR with the change: https://github.com/elastic/apm-agent-ruby/pull/507

Thanks – and thanks Felix for chiming in.

greyblake · August 26, 2019, 9:45am

Thank you! You're awesome!

system · September 16, 2019, 5:45am

This topic was automatically closed 20 days after the last reply. New replies are no longer allowed.