enhance destination fields for Oracle

[wolframhaussig]

Is your feature request related to a problem?

I was eager to see that the new 1.13.0 is finally out. I just tried it on our test system and would like a few enhancements:
We use a tomcat installation with multiple applications which are configured using property files. Our config for the JDBC connection typically looks like this:
app.jdbc.url=jdbc:oracle:thin:@(DESCRIPTION=(ADDRESS=(PROTOCOL=TCP)(HOST=localhost)(PORT=6203))(CONNECT_DATA=(SERVER=DEDICATED)(SERVICE_NAME=DB.FQDN.ORG.DE)))

The new agent creates the following destination fields:

 "destination": {
        "address": "(DESCRIPTION=(ADDRESS=(PROTOCOL=TCP)(HOST=localhost)(PORT=6203))(CONNECT_DATA=(SERVER=DEDICATED)(SERVICE_NAME=DB.FQDN.ORG.DE)))",
        "port": 1521,
        "service": {
          "resource": "oracle",
          "name": "oracle",
          "type": "db"
        }
      }

As the connection string can contain options which do not belong the destination server(e.g. SERVER=DEDICATED defines that the server will start a dedicated server process for our sessions) those strings might differ while still connecting to the same db.

Describe the solution you'd like

We would like to see the port parsed correctly(6203 instead of 1521) and the address of the destination handled correctly(I am open for discussions about the correct way :-) )
I also saw in the testcases that you do not currently extract the instance name - maybe this could be stored in the context.destination.service.resource?

Additional context

Syntax of the connect strings are defined here: https://docs.oracle.com/en/database/oracle/oracle-database/18/netrf/local-naming-parameters-in-tnsnames-ora-file.html

I don't know how hard it is to implement a solution for the destination adress which fits all - or most - users:

• only use SERVICE_NAME or SID as destination (would work for us)
  • what happens if both fields are present?
  • does not fit if SERVICE_NAME/SID is not globally unique(e.g. DB instead of DB.FQDN.DE)
• use the combination of HOST, PORT and SERVICE_NAME/SID
  • might create problems when applications running on the db server are connecting with HOST=localhost
  • might create problems when some applications are connecting with the hostname and some with the IP
  • what happens if multiple adresses are defined(failover)?
• make it configurable?

[eyalkoren]

@wolframhaussig thanks for your great feedback, again 🙂

I deferred adding these Oracle- and MySQL-specific syntaxes, but I guess there is no way around it (see my comment at the related PR). We will need to handle those. The destination.address should contain the actual host, localhost in this case.

Regarding the instance parsing and usage, I completely agree this is a valuable information, but we are currently not going to use it (see this comment and the responses to it).

A bit of background: the destination.service fields are there to support our upcoming service map feature. Since this is a WIP towards a first release, it is deliberately limited in what it attempts to support. In the first release, the resource field is going to be used only for discovery of connections between two instrumented services, and not for discovery of external services, such as databases.

Since these fields are saved for this purpose, if you want it as additional metadata to add context to your DB spans, the right way would probably be adding it as a field under context.db, but this is not currently planned. You may propose that in a different issue at the general APM repo, but let's keep this one as a bug to be fixed specifically in the Java agent.

[eyalkoren]

@wolframhaussig can you please verify the fix using the relevant artifact?

[wolframhaussig]

@eyalkoren it works! Thank you so much:

    "destination": {
      "address": "localhost",
      "port": 6203
    }

Looking forward to it :-)